This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix memory leak in python.c:do_start_initialization
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
>