[RFA-v2] Fix other memory leak in solib_target_current_sos

["Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>]

> -----Message d'origine-----
> De?: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé?: vendredi 14 décembre 2012 11:55
> À?: Pierre Muller
> Cc?: gdb-patches@sourceware.org
> Objet?: Re: [RFA] Fix other memory leak in solib_target_current_sos
> 
> On 12/14/2012 10:25 AM, Pierre Muller wrote:
> 
> >   I found another leak, down the same line,
> > solib_target_current_sos
> >   calls target_read_stralloc
> > which as its name suggests allocates the result on the heap...
> > But that string was not freed in the current code...
> >
> >   This patch fixes that leak.
> 
> Thanks.
> 
> > 2012-12-14  Pierre Muller  <muller@sourceware.org>
> >
> >         * solib-target.c (solib_target_current_sos): Remove 'const'
> >         qualifier from type of library_document local variable to be
> >         able to free it and avoid a memory leak.
> >
> > Index: src/gdb/solib-target.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/solib-target.c,v
> > retrieving revision 1.24
> > diff -u -p -r1.24 solib-target.c
> > --- src/gdb/solib-target.c      4 Jan 2012 08:17:11 -0000       1.24
> > +++ src/gdb/solib-target.c      14 Dec 2012 10:17:58 -0000
> > @@ -246,7 +246,7 @@ static struct so_list *
> >  solib_target_current_sos (void)
> >  {
> >    struct so_list *new_solib, *start = NULL, *last = NULL;
> > -  const char *library_document;
> > +  char *library_document;
> >    VEC(lm_info_p) *library_list;
> >    struct lm_info *info;
> >    int ix;
> > @@ -261,7 +261,10 @@ solib_target_current_sos (void)
> >    /* Parse the list.  */
> >    library_list = solib_target_parse_libraries (library_document);
> >    if (library_list == NULL)
> > -    return NULL;
> > +    {
> > +      xfree (library_document);
> > +      return NULL;
> > +    }
> >
> >    /* Build a struct so_list for each entry on the list.  */
> >    for (ix = 0; VEC_iterate (lm_info_p, library_list, ix, info); ix++)
> > @@ -291,6 +294,9 @@ solib_target_current_sos (void)
> >    /* Free the library list, but not its members.  */
> >    VEC_free (lm_info_p, library_list);
> >
> > +  /* Also free allocated library_document string.  */
> > +  xfree (library_document);
> > +
> >    return start;
> >  }
> >
> 
> Parsing XML, within solib_target_parse_libraries, may throw.  Thus, a
> cleanup would be better:
> 
> +  old_chain = make_cleanup (xfree, library_document);
>   library_list = solib_target_parse_libraries (library_document);
> +  do_cleanups (old_chain);

Of course...
  library_document isn't needed after that point anymore.

New version of the patch below.

Pierre

2012-12-14  Pierre Muller  <muller@sourceware.org>
	      Pedro Alves  <palves@redhat.com>

        * solib-target.c (solib_target_current_sos): Remove 'const'
        qualifier from type of library_document local variable to be
        able to free it and avoid a memory leak.
        Use cleanup chain to avoid leak even if exceptino is generated.


Index: solib-target.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-target.c,v
retrieving revision 1.24
diff -u -p -r1.24 solib-target.c
--- solib-target.c      4 Jan 2012 08:17:11 -0000       1.24
+++ solib-target.c      14 Dec 2012 13:03:13 -0000
@@ -246,7 +246,8 @@ static struct so_list *
 solib_target_current_sos (void)
 {
   struct so_list *new_solib, *start = NULL, *last = NULL;
-  const char *library_document;
+  char *library_document;
+  struct cleanup *old_chain;
   VEC(lm_info_p) *library_list;
   struct lm_info *info;
   int ix;
@@ -258,8 +259,15 @@ solib_target_current_sos (void)
   if (library_document == NULL)
     return NULL;

+  /* solib_target_parse_libraries may throw, so we use a cleanup.  */
+  old_chain = make_cleanup (xfree, library_document);
+
   /* Parse the list.  */
   library_list = solib_target_parse_libraries (library_document);
+
+  /* library_document string is not needed behind this point.  */
+  do_cleanups (old_chain);
+
   if (library_list == NULL)
     return NULL;