Re: [PATCH] solib_open, memory leak

[msnyder@sonic.net]

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

[Jim:]
> The code clearly deserves a comment like:
>
>     /* We try to find the library in various ways.  After each
>        attempt, either found_file >= 0 and temp_pathname is a malloc'd
>        string, or found_file < 0 and temp_pathname does not point to
>        storage that needs to be freed.  */
>
> In that light, it seems clearer to me to just put:
>
>    if (found_file < 0)
>      temp_pathname = NULL;
>    else
>      temp_pathname = xstrdup (temp_pathname);
>
> immediately after the open, to make it obvious that the rule is
> followed there.
>
[...]
> Don't you want those new lines to replace the two that followed them,
> not just precede them?

Yes, thanks.  How about this?


[-- Attachment #2: solib_open2.txt --]
[-- Type: text/plain, Size: 1692 bytes --]

Index: solib.c
===================================================================
RCS file: /cvs/src/src/gdb/solib.c,v
retrieving revision 1.94
diff -p -r1.94 solib.c
*** solib.c	3 Jul 2007 12:14:43 -0000	1.94
--- solib.c	8 Aug 2007 21:57:27 -0000
*************** solib_open (char *in_pathname, char **fo
*** 176,181 ****
--- 176,192 ----
    /* Now see if we can open it.  */
    found_file = open (temp_pathname, O_RDONLY | O_BINARY, 0);
  
+   /* We try to find the library in various ways.  After each attempt
+      (except for the one above), either found_file >= 0 and
+      temp_pathname is a malloc'd string, or found_file < 0 and
+      temp_pathname does not point to storage that needs to be
+      freed.  */
+ 
+     if (found_file < 0)
+       temp_pathname = NULL;
+     else
+       temp_pathname = xstrdup (temp_pathname);
+ 
    /* If the search in gdb_sysroot failed, and the path name is
       absolute at this point, make it relative.  (openp will try and open the
       file according to its absolute path otherwise, which is not what we want.)
*************** solib_open (char *in_pathname, char **fo
*** 224,231 ****
  
    /* Done.  If not found, tough luck.  Return found_file and 
       (optionally) found_pathname.  */
!   if (found_pathname != NULL && temp_pathname != NULL)
!     *found_pathname = xstrdup (temp_pathname);
    return found_file;
  }
  
--- 235,247 ----
  
    /* Done.  If not found, tough luck.  Return found_file and 
       (optionally) found_pathname.  */
!   if (temp_pathname)
!     {
!       if (found_pathname != NULL)
! 	*found_pathname = temp_pathname;
!       else
! 	xfree (temp_pathname);
!     }
    return found_file;
  }