RE: [patchv2 1/11] Remove xfullpath (use gdb_realpath instead)

["Agovic, Sanimir" <sanimir dot agovic at intel dot com>]

Hello Jan,

seems like this patch introduced a regression as pointed out on IRC. It is tracked via http://sourceware.org/bugzilla/show_bug.cgi?id=15415

Could you please have a look? Thanks. 

 -Sanimir

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf
> Of Jan Kratochvil
> Sent: Sunday, January 27, 2013 11:32 PM
> To: gdb-patches@sourceware.org
> Subject: [patchv2 1/11] Remove xfullpath (use gdb_realpath instead)
> 
> Hi,
> 
> according to Joel:
> 	Re: [patch 0/9] Absolute filenames
> 	http://sourceware.org/ml/gdb-patches/2013-01/msg00421.html
> 
> xfullpath can be removed, that is to replace it by gdb_realpath and remove any
> double gdb_realpath calls where appropriate.
> 
> gdb_realpath is like realpath while xfullpath was not resolving the very last
> filename component wrt symlinks.
> 
> 
> Thanks,
> Jan
> 
> 
>  gdb/cli/cli-cmds.c                  |  5 +----
>  gdb/dwarf2read.c                    | 31 +++++--------------------------
>  gdb/psymtab.c                       | 32 +++++---------------------------
>  gdb/source.c                        | 14 ++++----------
>  gdb/symfile.h                       |  5 ++---
>  gdb/symmisc.c                       |  2 +-
>  gdb/symtab.c                        | 43 ++++++-------------------------------------
>  gdb/symtab.h                        |  1 -
>  gdb/testsuite/gdb.gdb/xfullpath.exp | 14 +++++++-------
>  gdb/utils.c                         | 47 -----------------------------------------------
>  gdb/utils.h                         |  2 --
>  11 files changed, 31 insertions(+), 165 deletions(-)
>                                      ^^^
> 
> gdb/
> 2013-01-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Replace xfullpath calls by gdb_realpath calls.
> 	* cli/cli-cmds.c (find_and_open_script): Remove xfullpath from the
> 	function comment.
> 	* dwarf2read.c (dw2_map_expand_apply): Remove parameter full_path.
> 	Remove it from the iterate_over_some_symtabs call.
> 	(dw2_map_symtabs_matching_filename): Remove parameter full_path.
> 	Remove it from the dw2_map_expand_apply calls, remove a block handling
> 	it.
> 	* psymtab.c (partial_map_expand_apply): Remove parameter full_path.
> 	Remove it from the iterate_over_some_symtabs call.
> 	(partial_map_symtabs_matching_filename): Remove parameter full_path.
> 	Remove it from the partial_map_expand_apply calls, remove a block
> 	handling it.  Drop gdb_realpath call and cleanups from the real_path
> 	handling.
> 	* source.c (openp): Drop the comment part about xfullpath.  Replace
> 	xfullpath calls by gdb_realpath calls.
> 	(find_and_open_source): Replace xfullpath call by gdb_realpath call.
> 	* symfile.h (struct quick_symbol_functions): Remove parameter full_path
> 	from method map_symtabs_matching_filename and its comment.
> 	* symmisc.c (maintenance_print_msymbols): Replace xfullpath call by
> 	gdb_realpath call.
> 	* symtab.c (iterate_over_some_symtabs): Remove parameter full_path,
> 	remove it also from the function comment, remove a block handling it.
> 	Drop gdb_realpath call and cleanups from the real_path handling.
> 	(iterate_over_symtabs): Drop variable full_path and its use.
> 	* symtab.h (iterate_over_some_symtabs): Remove parameter full_path.
> 	* utils.c (xfullpath): Remove.
> 	* utils.h (xfullpath): Remove.
> 
> gdb/testsuite/
> 2013-01-25  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.gdb/xfullpath.exp: Replace xfullpath calls by gdb_realpath calls.
> 
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -465,10 +465,7 @@ show_script_ext_mode (struct ui_file *file, int from_tty,
>     we tried to open.
> 
>     If SEARCH_PATH is non-zero, and the file isn't found in cwd,
> -   search for it in the source search path.
> -
> -   NOTE: This calls openp which uses xfullpath to compute the full path
> -   instead of gdb_realpath.  Symbolic links are not resolved.  */
> +   search for it in the source search path.  */
> 
>  int
>  find_and_open_script (const char *script_file, int search_path,
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -3020,8 +3020,7 @@ dw2_forget_cached_source_info (struct objfile *objfile)
>  static int
>  dw2_map_expand_apply (struct objfile *objfile,
>  		      struct dwarf2_per_cu_data *per_cu,
> -		      const char *name,
> -		      const char *full_path, const char *real_path,
> +		      const char *name, const char *real_path,
>  		      int (*callback) (struct symtab *, void *),
>  		      void *data)
>  {
> @@ -3035,7 +3034,7 @@ dw2_map_expand_apply (struct objfile *objfile,
>       all of them.  */
>    dw2_instantiate_symtab (per_cu);
> 
> -  return iterate_over_some_symtabs (name, full_path, real_path, callback, data,
> +  return iterate_over_some_symtabs (name, real_path, callback, data,
>  				    objfile->symtabs, last_made);
>  }
> 
> @@ -3043,7 +3042,7 @@ dw2_map_expand_apply (struct objfile *objfile,
> 
>  static int
>  dw2_map_symtabs_matching_filename (struct objfile *objfile, const char *name,
> -				   const char *full_path, const char *real_path,
> +				   const char *real_path,
>  				   int (*callback) (struct symtab *, void *),
>  				   void *data)
>  {
> @@ -3077,8 +3076,7 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const
> char *name,
>  	  if (FILENAME_CMP (name, this_name) == 0
>  	      || (!is_abs && compare_filenames_for_search (this_name, name)))
>  	    {
> -	      if (dw2_map_expand_apply (objfile, per_cu,
> -					name, full_path, real_path,
> +	      if (dw2_map_expand_apply (objfile, per_cu, name, real_path,
>  					callback, data))
>  		return 1;
>  	    }
> @@ -3089,24 +3087,6 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const
> char *name,
>  	      && FILENAME_CMP (lbasename (this_name), name_basename) != 0)
>  	    continue;
> 
> -	  if (full_path != NULL)
> -	    {
> -	      const char *this_real_name = dw2_get_real_path (objfile,
> -							      file_data, j);
> -
> -	      if (this_real_name != NULL
> -		  && (FILENAME_CMP (full_path, this_real_name) == 0
> -		      || (!is_abs
> -			  && compare_filenames_for_search (this_real_name,
> -							   name))))
> -		{
> -		  if (dw2_map_expand_apply (objfile, per_cu,
> -					    name, full_path, real_path,
> -					    callback, data))
> -		    return 1;
> -		}
> -	    }
> -
>  	  if (real_path != NULL)
>  	    {
>  	      const char *this_real_name = dw2_get_real_path (objfile,
> @@ -3118,8 +3098,7 @@ dw2_map_symtabs_matching_filename (struct objfile *objfile, const
> char *name,
>  			  && compare_filenames_for_search (this_real_name,
>  							   name))))
>  		{
> -		  if (dw2_map_expand_apply (objfile, per_cu,
> -					    name, full_path, real_path,
> +		  if (dw2_map_expand_apply (objfile, per_cu, name, real_path,
>  					    callback, data))
>  		    return 1;
>  		}
> --- a/gdb/psymtab.c
> +++ b/gdb/psymtab.c
> @@ -131,7 +131,6 @@ require_partial_symbols (struct objfile *objfile, int verbose)
>  static int
>  partial_map_expand_apply (struct objfile *objfile,
>  			  const char *name,
> -			  const char *full_path,
>  			  const char *real_path,
>  			  struct partial_symtab *pst,
>  			  int (*callback) (struct symtab *, void *),
> @@ -151,7 +150,7 @@ partial_map_expand_apply (struct objfile *objfile,
>       all of them.  */
>    psymtab_to_symtab (objfile, pst);
> 
> -  return iterate_over_some_symtabs (name, full_path, real_path, callback, data,
> +  return iterate_over_some_symtabs (name, real_path, callback, data,
>  				    objfile->symtabs, last_made);
>  }
> 
> @@ -160,7 +159,6 @@ partial_map_expand_apply (struct objfile *objfile,
>  static int
>  partial_map_symtabs_matching_filename (struct objfile *objfile,
>  				       const char *name,
> -				       const char *full_path,
>  				       const char *real_path,
>  				       int (*callback) (struct symtab *,
>  							void *),
> @@ -184,7 +182,7 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
>      if (FILENAME_CMP (name, pst->filename) == 0
>  	|| (!is_abs && compare_filenames_for_search (pst->filename, name)))
>        {
> -	if (partial_map_expand_apply (objfile, name, full_path, real_path,
> +	if (partial_map_expand_apply (objfile, name, real_path,
>  				      pst, callback, data))
>  	  return 1;
>        }
> @@ -197,34 +195,14 @@ partial_map_symtabs_matching_filename (struct objfile *objfile,
> 
>      /* If the user gave us an absolute path, try to find the file in
>         this symtab and use its absolute path.  */
> -    if (full_path != NULL)
> -      {
> -	psymtab_to_fullname (pst);
> -	if (pst->fullname != NULL
> -	    && (FILENAME_CMP (full_path, pst->fullname) == 0
> -		|| (!is_abs && compare_filenames_for_search (pst->fullname,
> -							     name))))
> -	  {
> -	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
> -					  pst, callback, data))
> -	      return 1;
> -	  }
> -      }
> -
>      if (real_path != NULL)
>        {
> -        char *rp = NULL;
>  	psymtab_to_fullname (pst);
> -        if (pst->fullname != NULL)
> -          {
> -            rp = gdb_realpath (pst->fullname);
> -            make_cleanup (xfree, rp);
> -          }
> -	if (rp != NULL
> -	    && (FILENAME_CMP (real_path, rp) == 0
> +	if (pst->fullname != NULL
> +	    && (FILENAME_CMP (real_path, pst->fullname) == 0
>  		|| (!is_abs && compare_filenames_for_search (real_path, name))))
>  	  {
> -	    if (partial_map_expand_apply (objfile, name, full_path, real_path,
> +	    if (partial_map_expand_apply (objfile, name, real_path,
>  					  pst, callback, data))
>  	      return 1;
>  	  }
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -825,15 +825,11 @@ openp (const char *path, int opts, const char *string,
>  done:
>    if (filename_opened)
>      {
> -      /* If a file was opened, canonicalize its filename.  Use xfullpath
> -         rather than gdb_realpath to avoid resolving the basename part
> -         of filenames when the associated file is a symbolic link.  This
> -         fixes a potential inconsistency between the filenames known to
> -         GDB and the filenames it prints in the annotations.  */
> +      /* If a file was opened, canonicalize its filename.  */
>        if (fd < 0)
>  	*filename_opened = NULL;
>        else if (IS_ABSOLUTE_PATH (filename))
> -	*filename_opened = xfullpath (filename);
> +	*filename_opened = gdb_realpath (filename);
>        else
>  	{
>  	  /* Beware the // my son, the Emacs barfs, the botch that catch...  */
> @@ -843,7 +839,7 @@ done:
>  			    ? "" : SLASH_STRING,
>  			    filename, (char *)NULL);
> 
> -	  *filename_opened = xfullpath (f);
> +	  *filename_opened = gdb_realpath (f);
>  	  xfree (f);
>  	}
>      }
> @@ -986,9 +982,7 @@ find_and_open_source (const char *filename,
>        result = open (*fullname, OPEN_MODE);
>        if (result >= 0)
>  	{
> -	  /* Call xfullpath here to be consistent with openp
> -	     which we use below.  */
> -	  char *lpath = xfullpath (*fullname);
> +	  char *lpath = gdb_realpath (*fullname);
> 
>  	  xfree (*fullname);
>  	  *fullname = lpath;
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -160,8 +160,8 @@ struct quick_symbol_functions
>       where the source file is named NAME.
> 
>       If NAME is not absolute, a match after a '/' in the symbol
> -     table's file name will also work.  FULL_PATH is the absolute file
> -     name, and REAL_PATH is the same, run through gdb_realpath.
> +     table's file name will also work.  REAL_PATH is the absolute file
> +     name run through gdb_realpath.
> 
>       If a match is found, the "partial" symbol table is expanded.
>       Then, this calls iterate_over_some_symtabs (or equivalent) over
> @@ -169,7 +169,6 @@ struct quick_symbol_functions
>       The result of this call is returned.  */
>    int (*map_symtabs_matching_filename) (struct objfile *objfile,
>  					const char *name,
> -					const char *full_path,
>  					const char *real_path,
>  					int (*callback) (struct symtab *,
>  							 void *),
> --- a/gdb/symmisc.c
> +++ b/gdb/symmisc.c
> @@ -655,7 +655,7 @@ maintenance_print_msymbols (char *args, int from_tty)
>        /* If a second arg is supplied, it is a source file name to match on.  */
>        if (argv[1] != NULL)
>  	{
> -	  symname = xfullpath (argv[1]);
> +	  symname = gdb_realpath (argv[1]);
>  	  make_cleanup (xfree, symname);
>  	  if (symname && stat (symname, &sym_st))
>  	    perror_with_name (symname);
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -179,7 +179,7 @@ compare_filenames_for_search (const char *filename, const char
> *search_name)
>  /* Check for a symtab of a specific name by searching some symtabs.
>     This is a helper function for callbacks of iterate_over_symtabs.
> 
> -   The return value, NAME, FULL_PATH, REAL_PATH, CALLBACK, and DATA
> +   The return value, NAME, REAL_PATH, CALLBACK, and DATA
>     are identical to the `map_symtabs_matching_filename' method of
>     quick_symbol_functions.
> 
> @@ -189,7 +189,6 @@ compare_filenames_for_search (const char *filename, const char
> *search_name)
> 
>  int
>  iterate_over_some_symtabs (const char *name,
> -			   const char *full_path,
>  			   const char *real_path,
>  			   int (*callback) (struct symtab *symtab,
>  					    void *data),
> @@ -225,47 +224,21 @@ iterate_over_some_symtabs (const char *name,
>      /* If the user gave us an absolute path, try to find the file in
>         this symtab and use its absolute path.  */
> 
> -    if (full_path != NULL)
> -      {
> -        const char *fp = symtab_to_fullname (s);
> -
> -        if (FILENAME_CMP (full_path, fp) == 0)
> -          {
> -	    if (callback (s, data))
> -	      return 1;
> -          }
> -
> -	if (!is_abs && compare_filenames_for_search (fp, name))
> -	  {
> -	    if (callback (s, data))
> -	      return 1;
> -	  }
> -      }
> -
>      if (real_path != NULL)
>        {
>          const char *fullname = symtab_to_fullname (s);
> -	char *rp = gdb_realpath (fullname);
> -	struct cleanup *cleanups = make_cleanup (xfree, rp);
> 
> -	if (FILENAME_CMP (real_path, rp) == 0)
> +	if (FILENAME_CMP (real_path, fullname) == 0)
>  	  {
>  	    if (callback (s, data))
> -	      {
> -		do_cleanups (cleanups);
> -		return 1;
> -	      }
> +	      return 1;
>  	  }
> 
> -	if (!is_abs && compare_filenames_for_search (rp, name))
> +	if (!is_abs && compare_filenames_for_search (fullname, name))
>  	  {
>  	    if (callback (s, data))
> -	      {
> -		do_cleanups (cleanups);
> -		return 1;
> -	      }
> +	      return 1;
>  	  }
> -	do_cleanups (cleanups);
>        }
>      }
> 
> @@ -288,22 +261,19 @@ iterate_over_symtabs (const char *name,
>    struct symtab *s = NULL;
>    struct objfile *objfile;
>    char *real_path = NULL;
> -  char *full_path = NULL;
>    struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
> 
>    /* Here we are interested in canonicalizing an absolute path, not
>       absolutizing a relative path.  */
>    if (IS_ABSOLUTE_PATH (name))
>      {
> -      full_path = xfullpath (name);
> -      make_cleanup (xfree, full_path);
>        real_path = gdb_realpath (name);
>        make_cleanup (xfree, real_path);
>      }
> 
>    ALL_OBJFILES (objfile)
>    {
> -    if (iterate_over_some_symtabs (name, full_path, real_path, callback, data,
> +    if (iterate_over_some_symtabs (name, real_path, callback, data,
>  				   objfile->symtabs, NULL))
>        {
>  	do_cleanups (cleanups);
> @@ -319,7 +289,6 @@ iterate_over_symtabs (const char *name,
>      if (objfile->sf
>  	&& objfile->sf->qf->map_symtabs_matching_filename (objfile,
>  							   name,
> -							   full_path,
>  							   real_path,
>  							   callback,
>  							   data))
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -1284,7 +1284,6 @@ int compare_filenames_for_search (const char *filename,
>  				  const char *search_name);
> 
>  int iterate_over_some_symtabs (const char *name,
> -			       const char *full_path,
>  			       const char *real_path,
>  			       int (*callback) (struct symtab *symtab,
>  						void *data),
> --- a/gdb/testsuite/gdb.gdb/xfullpath.exp
> +++ b/gdb/testsuite/gdb.gdb/xfullpath.exp
> @@ -103,37 +103,37 @@ proc test_with_self { executable } {
>      }
> 
>      # A file which contains a directory prefix
> -    gdb_test "print xfullpath (\"./xfullpath.exp\")" \
> +    gdb_test "print gdb_realpath (\"./xfullpath.exp\")" \
>               ".\[0-9\]+ =.*\".*/xfullpath.exp\"" \
>               "A filename with ./ as the directory prefix"
> 
>      # A file which contains a directory prefix
> -    gdb_test "print xfullpath (\"../../defs.h\")" \
> +    gdb_test "print gdb_realpath (\"../../defs.h\")" \
>               ".\[0-9\]+ =.*\".*/defs.h\"" \
>               "A filename with ../ in the directory prefix"
> 
>      # A one-character filename
> -    gdb_test "print xfullpath (\"./a\")" \
> +    gdb_test "print gdb_realpath (\"./a\")" \
>               ".\[0-9\]+ =.*\".*/a\"" \
>               "A one-char filename in the current directory"
> 
>      # A file in the root directory
> -    gdb_test "print xfullpath (\"/root_file_which_should_exist\")" \
> +    gdb_test "print gdb_realpath (\"/root_file_which_should_exist\")" \
>               ".\[0-9\]+ =.*\"/root_file_which_should_exist\"" \
>               "A filename in the root directory"
> 
>      # A file which does not have a directory prefix
> -    gdb_test "print xfullpath (\"xfullpath.exp\")" \
> +    gdb_test "print gdb_realpath (\"xfullpath.exp\")" \
>               ".\[0-9\]+ =.*\"xfullpath.exp\"" \
>               "A filename without any directory prefix"
> 
>      # A one-char filename without any directory prefix
> -    gdb_test "print xfullpath (\"a\")" \
> +    gdb_test "print gdb_realpath (\"a\")" \
>               ".\[0-9\]+ =.*\"a\"" \
>               "A one-char filename without any directory prefix"
> 
>      # An empty filename
> -    gdb_test "print xfullpath (\"\")" \
> +    gdb_test "print gdb_realpath (\"\")" \
>               ".\[0-9\]+ =.*\"\"" \
>               "An empty filename"
> 
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -3286,53 +3286,6 @@ gdb_realpath (const char *filename)
>    return xstrdup (filename);
>  }
> 
> -/* Return a copy of FILENAME, with its directory prefix canonicalized
> -   by gdb_realpath.  */
> -
> -char *
> -xfullpath (const char *filename)
> -{
> -  const char *base_name = lbasename (filename);
> -  char *dir_name;
> -  char *real_path;
> -  char *result;
> -
> -  /* Extract the basename of filename, and return immediately
> -     a copy of filename if it does not contain any directory prefix.  */
> -  if (base_name == filename)
> -    return xstrdup (filename);
> -
> -  dir_name = alloca ((size_t) (base_name - filename + 2));
> -  /* Allocate enough space to store the dir_name + plus one extra
> -     character sometimes needed under Windows (see below), and
> -     then the closing \000 character.  */
> -  strncpy (dir_name, filename, base_name - filename);
> -  dir_name[base_name - filename] = '\000';
> -
> -#ifdef HAVE_DOS_BASED_FILE_SYSTEM
> -  /* We need to be careful when filename is of the form 'd:foo', which
> -     is equivalent of d:./foo, which is totally different from d:/foo.  */
> -  if (strlen (dir_name) == 2 && isalpha (dir_name[0]) && dir_name[1] == ':')
> -    {
> -      dir_name[2] = '.';
> -      dir_name[3] = '\000';
> -    }
> -#endif
> -
> -  /* Canonicalize the directory prefix, and build the resulting
> -     filename.  If the dirname realpath already contains an ending
> -     directory separator, avoid doubling it.  */
> -  real_path = gdb_realpath (dir_name);
> -  if (IS_DIR_SEPARATOR (real_path[strlen (real_path) - 1]))
> -    result = concat (real_path, base_name, (char *) NULL);
> -  else
> -    result = concat (real_path, SLASH_STRING, base_name, (char *) NULL);
> -
> -  xfree (real_path);
> -  return result;
> -}
> -
> -
>  /* This is the 32-bit CRC function used by the GNU separate debug
>     facility.  An executable may contain a section named
>     .gnu_debuglink, which holds the name of a separate executable file
> --- a/gdb/utils.h
> +++ b/gdb/utils.h
> @@ -124,8 +124,6 @@ extern struct cleanup *make_bpstat_clear_actions_cleanup (void);
> 
>  extern char *gdb_realpath (const char *);
> 
> -extern char *xfullpath (const char *);
> -
>  extern int gdb_filename_fnmatch (const char *pattern, const char *string,
>  				 int flags);
> 
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052