Re: RFA: Use lrealpath instead of gdb_realpath

Eli Zaretskii
2005-05-29T08:37:15Z

> 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.