lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] [lmi-commits] master a67848a 1/3: Use 'new(wx)' to allocate me


From: Vadim Zeitlin
Subject: Re: [lmi] [lmi-commits] master a67848a 1/3: Use 'new(wx)' to allocate memory that will be freed by wx
Date: Sun, 7 Feb 2021 00:00:18 +0100

On Sat,  6 Feb 2021 14:47:06 -0500 (EST) Greg Chicares 
<gchicares@sbcglobal.net> wrote:

GC> branch: master
GC> commit a67848aded268036c620da89601ef47ed8e8e9b1
GC> Author: Gregory W. Chicares <gchicares@sbcglobal.net>
GC> Commit: Gregory W. Chicares <gchicares@sbcglobal.net>
GC> 
GC>     Use 'new(wx)' to allocate memory that will be freed by wx
GC>     
GC>     s/new/new(wx)/ in appropriate cases that had been overlooked.

 Thanks for fixing this and sorry for overlooking this so many times!

GC> diff --git a/skeleton.cpp b/skeleton.cpp
GC> index 18e4494..22e96d3 100644
GC> --- a/skeleton.cpp
GC> +++ b/skeleton.cpp
GC> @@ -1081,6 +1081,9 @@ void 
Skeleton::UponTestFloatingPointEnvironment(wxCommandEvent&)
GC>  
GC>  void Skeleton::UponTestPasting(wxCommandEvent&)
GC>  {
GC> +    // This uses 'new' rather than 'new(wx)' because the object is
GC> +    // explicitly deleted by calling Destroy(); yet the Destroy()
GC> +    // call isn't reached if an exception is thrown.
GC>      InputSequenceEntry* z = new InputSequenceEntry(frame_, wxID_ANY, 
"Testing...");
GC>      LMI_ASSERT(z);
GC>      wxTextCtrl& t = z->text_ctrl();

 I'm not sure about the logic here however: in either case this pointer is
deleted by wxWidgets, either from inside Destroy() or when deleting the
parent frame later. So it seems that it should be still allocated with
new(wx).

 Independently of this, we probably ought to use some kind of scope guard
to ensure that Destroy() is called in any case, as even if there is no real
memory leak here (because the InputSequenceEntry will eventually be deleted
by the frame), there is a "window leak", as the entry window will remain
orphaned on the screen. Or we could just use a unique_ptr as Destroy() for
a child window is the same as deleting it anyhow.

 Finally, we could remove LMI_ASSERT(z) in any case, as neither new nor
new(wx) will ever return a null pointer.

 Regards,
VZ

Attachment: pgpmVkSVBfYPl.pgp
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]