Re: [Nano-devel] Memory leak in browser

[Benno Schulenberg]

On Wed, Feb 24, 2016, at 18:36, Rishabh Dave wrote:
> > > +               temp = striponedir(filelist[selected]);
> > > +               prev_dir = mallocstrcpy(NULL, temp);
> >
> > Why not simply free 'prev_dir' at the end of the routine?
> 
> Won't that lead to a memory leak? To reassign the address of memory full of
> new string, we must first un-assign the address of memory full of old
> string. I tried valgrind, we will have leaks.

Well, what I meant was: look at striponedir().  The thing that
it returns is already a mallocstrcopied copy of the given path.
Why mallocstrcopy it again?  Just elide the whole cpy() call.

(The mistake was made in revision 3707 -- when the mallocstrassn()
was removed, also the mallocstrcpy() should have been snipped.)

However, there is one small chance that trying to open the
returned dir will fail (because someone renamed or deleted it
while we were browsing), and in that case prev_dir would be
leaked.

So I have applied the attached patch to SVN, r5676.
And then followed it up with the proper fix in r5677.


> Inline assignment, done and 'temp' is declared and freed locally. I have
> attached a patch, in case it does lead to memory leak. :)

Ouch.  You were told that inline assignments are *bad*,
and now you do it again.  :(

Benno

-- 
http://www.fastmail.com - Choose from over 50 domains or use your own

freeprevdir.patch
Description: Text Data