Re: RFA: Use lrealpath instead of gdb_realpath

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sat, 28 May 2005 19:42:34 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Christopher Faylor <me@cgf.cx>
> 
> Eli, I understand that you think this is a bad choice.  However, the mingw32
> maintainers disagree.  My only motivation here is to get rid of the
> duplicated function

As your patch clearly shows, gdb_realpath is _not_ identical to
lrealpath.  For example, it doesn't have the Windows code that calls
GetFullPathName.  So we are not replacing a duplicated function, not
really.

Let me explain my concerns about using lrealpath in its current shape:

 . The Windows code in lrealpath converts all forward slashes to
   backslashes, which I think is a bad idea.  By contrast, the current
   gdb_realpath would leave existing forward slashes alone.  That is
   an incompatible change, and my gut feeling is that we should not
   risk it.  On top of that, I think we always use '/' to generate
   file names, so with this change we will produce ugly file names
   like D:\foo\bar/baz.c.

 . The Windows code in lrealpath downcases the file name returned by
   GetFullPathName.  That might be okay for when we need a canonical
   file name suitable for comparison by strcmp, but in the case of GDB
   it is IMHO a misservice to the user, since (a) we correctly use
   FILENAME_CMP to compare file names, and (b) presenting downcased
   file names to the user is a gratuitous ugliness that we should try
   to avoid.

 . GetFullPathName is documented to work even if the path and/or the
   file name do not exist.  By contrast, both realpath and
   canonicalize_file_name are documented to fail if some or all of the
   components of the resulting file name do not exist.  Thus, I submit
   that the Windows code is not a faithful emulation of the Posix
   code, and thus could cause subtle bugs/misfeatures.

 . lrealpath uses strdup while gdb_realpath uses xstrdup.  Thus, if we
   use lrealpath, we will behave differently when we run out of
   memory.

For these reasons, I believe we should not switch to lrealpath until
we address these issues.  We could resolve them either by suitable
modifications to lrealpath (but then we would need to verify that
other users of lrealpath don't suffer; e.g., I suspect that the
downcasing part is there because some program uses strcmp to compare
file names), or by retaining gdb_realpath with suitable changes to
support the MinGW build.

> --- symtab.c	8 Mar 2005 04:34:44 -0000	1.145
> +++ symtab.c	28 May 2005 23:13:40 -0000
> @@ -163,7 +163,7 @@ lookup_symtab (const char *name)
>      {
>        full_path = xfullpath (name);
>        make_cleanup (xfree, full_path);
> -      real_path = gdb_realpath (name);
> +      real_path = lrealpath (name);
>        make_cleanup (xfree, real_path);
>      }

Can we please talk about this snippet?  I don't understand the need
for calling both xfullpath and lrealpath here.  If that's because
lrealpath might fail if the basename does not exist, I think we should
call lrealpath first and fall back on xfullpath only after that.