[RFA] source.c:find_and_open_source cleanup

[RFA] source.c:find_and_open_source cleanup

[Doug Evans]

This is odd:

  if (result >= 0)
    {
      char *tmp_fullname;
      tmp_fullname = *fullname;
      *fullname = xstrdup (tmp_fullname);
      xfree (tmp_fullname);
    }

Is there something subtle going on here?
I thought maybe the caller might be expecting a newly allocated value
if it passed in a value for `fullname', but the previous value has
already been freed at this point.

2008-04-07  Doug Evans  <dje@google.com>

	* source.c (find_and_open_source): Add some comments clarifying
	handling of FULLNAME argument.	Make static.  Remove pointless
	xstrdup/xfree.

Index: source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.86
diff -u -p -u -p -r1.86 source.c
--- source.c	14 Mar 2008 18:39:43 -0000	1.86
+++ source.c	7 Apr 2008 19:30:01 -0000
@@ -927,15 +927,19 @@ rewrite_source_path (const char *path)
    DIRNAME is the compilation directory of a particular source file.
            Only some debug formats provide this info.
    FULLNAME can be the last known absolute path to the file in question.
+     Space for the path must have been malloc'd.  If a path substitution
+     is applied we free the old value and set a new one.
 
    On Success 
      A valid file descriptor is returned. ( the return value is positive )
      FULLNAME is set to the absolute path to the file just opened.
+     The caller is responsible for freeing FULLNAME.
 
    On Failure
      An invalid file descriptor is returned. ( the return value is negative ) 
      FULLNAME is set to NULL.  */
-int
+
+static int
 find_and_open_source (struct objfile *objfile,
 		      const char *filename,
 		      const char *dirname,
@@ -1022,13 +1026,6 @@ find_and_open_source (struct objfile *ob
 	result = openp (path, OPF_SEARCH_IN_PATH, p, OPEN_MODE, 0, fullname);
     }
 
-  if (result >= 0)
-    {
-      char *tmp_fullname;
-      tmp_fullname = *fullname;
-      *fullname = xstrdup (tmp_fullname);
-      xfree (tmp_fullname);
-    }
   return result;
 }
 

Re: [RFA] source.c:find_and_open_source cleanup

[Doug Evans]

Presumably this is just leftovers from the mstrsave removal.  The code
was originally something like:

  if (result >= 0)
    {
      char *tmp_fullname;
      tmp_fullname = *fullname;
      *fullname = mstrsave (objfile->md, *fullname);
      xfree (tmp_fullname);
    }


On Mon, Apr 7, 2008 at 12:34 PM, Doug Evans <dje@google.com> wrote:
> This is odd:
>
>   if (result >= 0)
>     {
>       char *tmp_fullname;
>       tmp_fullname = *fullname;
>       *fullname = xstrdup (tmp_fullname);
>       xfree (tmp_fullname);
>     }
>
>  Is there something subtle going on here?
>  I thought maybe the caller might be expecting a newly allocated value
>  if it passed in a value for `fullname', but the previous value has
>  already been freed at this point.
>
>  2008-04-07  Doug Evans  <dje@google.com>
>
>         * source.c (find_and_open_source): Add some comments clarifying
>         handling of FULLNAME argument.  Make static.  Remove pointless
>         xstrdup/xfree.
>
>  Index: source.c
>  ===================================================================
>  RCS file: /cvs/src/src/gdb/source.c,v
>  retrieving revision 1.86
>  diff -u -p -u -p -r1.86 source.c
>  --- source.c    14 Mar 2008 18:39:43 -0000      1.86
>  +++ source.c    7 Apr 2008 19:30:01 -0000
>  @@ -927,15 +927,19 @@ rewrite_source_path (const char *path)
>     DIRNAME is the compilation directory of a particular source file.
>             Only some debug formats provide this info.
>     FULLNAME can be the last known absolute path to the file in question.
>  +     Space for the path must have been malloc'd.  If a path substitution
>  +     is applied we free the old value and set a new one.
>
>     On Success
>       A valid file descriptor is returned. ( the return value is positive )
>       FULLNAME is set to the absolute path to the file just opened.
>  +     The caller is responsible for freeing FULLNAME.
>
>     On Failure
>       An invalid file descriptor is returned. ( the return value is negative )
>       FULLNAME is set to NULL.  */
>  -int
>  +
>  +static int
>   find_and_open_source (struct objfile *objfile,
>                       const char *filename,
>                       const char *dirname,
>  @@ -1022,13 +1026,6 @@ find_and_open_source (struct objfile *ob
>         result = openp (path, OPF_SEARCH_IN_PATH, p, OPEN_MODE, 0, fullname);
>      }
>
>  -  if (result >= 0)
>  -    {
>  -      char *tmp_fullname;
>  -      tmp_fullname = *fullname;
>  -      *fullname = xstrdup (tmp_fullname);
>  -      xfree (tmp_fullname);
>  -    }
>    return result;
>   }
>
>

Re: [RFA] source.c:find_and_open_source cleanup

[Daniel Jacobowitz]

On Mon, Apr 07, 2008 at 12:34:12PM -0700, Doug Evans wrote:
> This is odd:
> 
>   if (result >= 0)
>     {
>       char *tmp_fullname;
>       tmp_fullname = *fullname;
>       *fullname = xstrdup (tmp_fullname);
>       xfree (tmp_fullname);
>     }
> 
> Is there something subtle going on here?
> I thought maybe the caller might be expecting a newly allocated value
> if it passed in a value for `fullname', but the previous value has
> already been freed at this point.
> 
> 2008-04-07  Doug Evans  <dje@google.com>
> 
> 	* source.c (find_and_open_source): Add some comments clarifying
> 	handling of FULLNAME argument.	Make static.  Remove pointless
> 	xstrdup/xfree.

Looks good to me.  Please commit, thanks!

-- 
Daniel Jacobowitz
CodeSourcery