Re: [PATCH] Fix memory leak in python.c:do_start_initialization

[Philipp Rudo <prudo@linux.vnet.ibm.com>]

On Wed, 22 Mar 2017 15:19:16 +0000
Pedro Alves <palves@redhat.com> wrote:

> Hi Philipp,
> 
> Thanks.
> 
> On 03/22/2017 01:11 PM, Philipp Rudo wrote:
> 
> > diff --git a/gdb/python/python.c b/gdb/python/python.c
> > index 73fb3d0..6b16613 100644
> > --- a/gdb/python/python.c
> > +++ b/gdb/python/python.c
> > @@ -1535,7 +1535,7 @@ extern initialize_file_ftype
> > _initialize_python; static bool
> >  do_start_initialization ()
> >  {
> > -  char *progname;
> > +  char *progname, *libdir;
> >  #ifdef IS_PY3K
> >    int i;
> >    size_t progsize, count;
> > @@ -1550,8 +1550,10 @@ do_start_initialization ()
> >       /foo/bin/python
> >       /foo/lib/pythonX.Y/...
> >       This must be done before calling Py_Initialize.  */
> > -  progname = concat (ldirname (python_libdir), SLASH_STRING, "bin",
> > +  libdir = ldirname (python_libdir);
> > +  progname = concat (libdir, SLASH_STRING, "bin",
> >  		     SLASH_STRING, "python", (char *) NULL);
> > +  xfree (libdir);  
> 
> Let's restrict the new variable to the #if block that needs it.
> I.e., declare the variable where is initialized, like:
> 
>   const char *libdir = ldirname (python_libdir);
>   progname = concat (libdir, SLASH_STRING, "bin",
> 
> OK with that change.  Please push.

Shall I fix it? Or do we go with your ldirname fix?

> 
> Note, you could have used reconcat instead of concat, avoiding the
> xfree call, and maybe one reallocation, but that's hardly an
> issue here.

Thought about using reconcat.  But in the end reconcat would have done
the same -- calling free on libdir.  Thats why I decided it would be
better readable when not hidden.  

> Perhaps better overall would be to make ldirname return a std::string
> and eliminate these leaks "by design".  It'd get rid of several
> make_cleanup calls throughout too.  I'll give that a quick try.

Or maybe combining all path handling in a 'class gdbpath'.  But
make ldirname return std::string definitely is an advantage.

> 
> Thanks,
> Pedro Alves
>