Re: Array<T>::nil_rep leaks memory

[Jaroslav Hajek]

On Mon, Sep 22, 2008 at 10:02 PM, John W. Eaton <address@hidden> wrote:
> On 19-Sep-2008, Jaroslav Hajek wrote:
>
> | in the function Array<T>::nil_rep a static pointer to an ArrayRep is
> | declared and allocated via new. Unless I'm mistaken, such a pointer is
> | never deleted and causes a memory leak. The attached patch fixes this
> | by using a static variable instead and returning its address.
>
> | # HG changeset patch
> | # User Jaroslav Hajek <address@hidden>
> | # Date 1221808946 -7200
> | # Node ID 45b534558b2276b9fc85c5f1871102e199942acf
> | # Parent  31e86163b7529285a16092625c3ae8ec0d986f92
> | fix memory leak in Array
> |
> | diff --git a/liboctave/Array.h b/liboctave/Array.h
> | --- a/liboctave/Array.h
> | +++ b/liboctave/Array.h
> | @@ -163,10 +163,9 @@
> |
> |    typename Array<T>::ArrayRep *nil_rep (void) const
> |      {
> | -      static typename Array<T>::ArrayRep *nr
> | -     = new typename Array<T>::ArrayRep ();
> | +      static typename Array<T>::ArrayRep nr;
> |
> | -      return nr;
> | +      return &nr;
> |      }
> |
> |    template <class U>
> | diff --git a/liboctave/ChangeLog b/liboctave/ChangeLog
> | --- a/liboctave/ChangeLog
> | +++ b/liboctave/ChangeLog
> | @@ -0,0 +1,5 @@
> | +2008-09-19  Jaroslav Hajek  <address@hidden>
> | +
> | +     * Array.h (Array<T>::nil_rep): Use static variable instead of
> | +     allocating static pointer.
> | +
>
> The object pointed to by Array<T>::rep should be allocated by new,
> otherwise calling delete on it in the destructor would be an error.
> So I think this code only works because delete is never called for the
> value returned by nil_rep.

Yes, precisely. No Array<T> object is responsible for destroying the
static object. It is destroyed at the end of the program.

> So does it really matter?  It seems more
> correct to me that the Array<T>::rep pointer should always point to an
> object allocated with new.

Well, as much as I understand it, the static object will be correctly
deallocated at the end of the program, while the new-allocated object
(on heap, presumably) is *never* freed. It's not much of an issue, but
it is a memory leak.

>
> jwe
>



-- 
RNDr. Jaroslav Hajek
computing expert
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz