Better realpath

Better realpath

[Vladimir Prus]

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


GDB has a function to get real path of a file, gdb_realpath. Unfortunately,
that function is essentially a copy-paste of libiberty's lrealpath, with
the extra bonus that gdb_realpath *does not* have any Windows-specific
code. As result, GDB is not capable to simplify ".." in windows paths,
and among other problems, breakpoints set using full file names containing
".." will not work.

This patch makes GDB use libibery's lrealpath. OK?

- Volodya

       gdb/
       * utils.c (gdb_realpath): Use lrealpath.

[-- Attachment #2: commit.diff --]
[-- Type: text/x-diff, Size: 2723 bytes --]

Index: gdb/utils.c
===================================================================
--- gdb/utils.c	(revision 211706)
+++ gdb/utils.c	(revision 211707)
@@ -2872,73 +2872,10 @@
 char *
 gdb_realpath (const char *filename)
 {
-  /* Method 1: The system has a compile time upper bound on a filename
-     path.  Use that and realpath() to canonicalize the name.  This is
-     the most common case.  Note that, if there isn't a compile time
-     upper bound, you want to avoid realpath() at all costs.  */
-#if defined(HAVE_REALPATH)
-  {
-# if defined (PATH_MAX)
-    char buf[PATH_MAX];
-#  define USE_REALPATH
-# elif defined (MAXPATHLEN)
-    char buf[MAXPATHLEN];
-#  define USE_REALPATH
-# endif
-# if defined (USE_REALPATH)
-    const char *rp = realpath (filename, buf);
-    if (rp == NULL)
-      rp = filename;
-    return xstrdup (rp);
-# endif
-  }
-#endif /* HAVE_REALPATH */
-
-  /* Method 2: The host system (i.e., GNU) has the function
-     canonicalize_file_name() which malloc's a chunk of memory and
-     returns that, use that.  */
-#if defined(HAVE_CANONICALIZE_FILE_NAME)
-  {
-    char *rp = canonicalize_file_name (filename);
-    if (rp == NULL)
-      return xstrdup (filename);
-    else
-      return rp;
-  }
-#endif
-
-  /* FIXME: cagney/2002-11-13:
-
-     Method 2a: Use realpath() with a NULL buffer.  Some systems, due
-     to the problems described in in method 3, have modified their
-     realpath() implementation so that it will allocate a buffer when
-     NULL is passed in.  Before this can be used, though, some sort of
-     configure time test would need to be added.  Otherwize the code
-     will likely core dump.  */
-
-  /* Method 3: Now we're getting desperate!  The system doesn't have a
-     compile time buffer size and no alternative function.  Query the
-     OS, using pathconf(), for the buffer limit.  Care is needed
-     though, some systems do not limit PATH_MAX (return -1 for
-     pathconf()) making it impossible to pass a correctly sized buffer
-     to realpath() (it could always overflow).  On those systems, we
-     skip this.  */
-#if defined (HAVE_REALPATH) && defined (HAVE_UNISTD_H) && defined(HAVE_ALLOCA)
-  {
-    /* Find out the max path size.  */
-    long path_max = pathconf ("/", _PC_PATH_MAX);
-    if (path_max > 0)
-      {
-	/* PATH_MAX is bounded.  */
-	char *buf = alloca (path_max);
-	char *rp = realpath (filename, buf);
-	return xstrdup (rp ? rp : filename);
-      }
-  }
-#endif
-
-  /* This system is a lost cause, just dup the buffer.  */
-  return xstrdup (filename);
+  char *r = lrealpath (filename);
+  if (!r)
+    nomem (0);
+  return r;
 }
 
 /* Return a copy of FILENAME, with its directory prefix canonicalized

RE: Better realpath

[Pierre Muller]

I just looked at your patch and the idea seems
good... But I am no expert on this field.

  One question though, libiberty lrealpath
function uses strdup, while the replaced 
code uses xstrdup.
  xstrdup is also defined in libiberty
as being a replacement of strdup that never
fails... (didn't really get how this is
possible...)

  Anyhow, is this a potential issue?


Pierre Muller
Pascal language support maintainer for GDB



-----Message d'origine-----
De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] De la part de Vladimir Prus
Envoyé : Saturday, June 14, 2008 8:25 AM
À : gdb-patches@sources.redhat.com
Objet : Better realpath


GDB has a function to get real path of a file, gdb_realpath. Unfortunately, that function is essentially a copy-paste of libiberty's lrealpath, with the extra bonus that gdb_realpath *does not* have any Windows-specific code. As result, GDB is not capable to simplify ".." in windows paths, and among other problems, breakpoints set using full file names containing ".." will not work.

This patch makes GDB use libibery's lrealpath. OK?

- Volodya

       gdb/
       * utils.c (gdb_realpath): Use lrealpath.

Re: Better realpath

[Vladimir Prus]

On Saturday 14 June 2008 14:31:32 Pierre Muller wrote:
> I just looked at your patch and the idea seems
> good... But I am no expert on this field.
> 
>   One question though, libiberty lrealpath
> function uses strdup, while the replaced 
> code uses xstrdup.
>   xstrdup is also defined in libiberty
> as being a replacement of strdup that never
> fails... (didn't really get how this is
> possible...)
> 
>   Anyhow, is this a potential issue?

I don't think so. xstrdup is supposed to fail if it cannot get enough
memory. lrealpath, as it is now, will return NULL, but with my patch, 
there's a check for that in gdb_realpath -- and a call to nomem.

(And no, I did not think about that myself -- Dan suggested the
check and the call to nomem).

- Volodya

Re: Better realpath

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 14 Jun 2008 10:24:41 +0400
> 
> GDB has a function to get real path of a file, gdb_realpath. Unfortunately,
> that function is essentially a copy-paste of libiberty's lrealpath, with
> the extra bonus that gdb_realpath *does not* have any Windows-specific
> code. As result, GDB is not capable to simplify ".." in windows paths,
> and among other problems, breakpoints set using full file names containing
> ".." will not work.
> 
> This patch makes GDB use libibery's lrealpath. OK?

There were past discussions about this; please read them before
concluding that lrealpath is all we need.  My records seem to indicate
that a thread Re "fullname attribute for GDB/MI stack frames" in May
2005 on this list is one of them, but maybe there were more.  You will
find my critique of what lrealpath does on Windows in a message in
that thread I sent on May 29, and suggested ways to improve it in a
followup message on the same day.  I think if we are going to use
lrealpath, we should at least make its behavior consistent and correct
on all supported platforms, including native Windows (i.e. MinGW).

Re: Better realpath

[Vladimir Prus]

On Saturday 14 June 2008 15:28:01 Eli Zaretskii wrote:
> > From: Vladimir Prus <vladimir@codesourcery.com>
> > Date: Sat, 14 Jun 2008 10:24:41 +0400
> > 
> > GDB has a function to get real path of a file, gdb_realpath. Unfortunately,
> > that function is essentially a copy-paste of libiberty's lrealpath, with
> > the extra bonus that gdb_realpath *does not* have any Windows-specific
> > code. As result, GDB is not capable to simplify ".." in windows paths,
> > and among other problems, breakpoints set using full file names containing
> > ".." will not work.
> > 
> > This patch makes GDB use libibery's lrealpath. OK?
> 
> There were past discussions about this; please read them before
> concluding that lrealpath is all we need.  

I'm not concluding this, I'm concluding it is strictly better than
gdb_realpath in its current form. And given that currently, trying
to set a breakpoint using full path with ".." plain does not work,
I think the proposed patch is definitely an improvement.

The implications of the current behaviour are much more grave than it
seems. Presently, Eclipse uses file basename to set breakpoints by
default, which obviously breaks badly if there two files with the same
basename in the project, which is not uncommon. And if we try to use
full paths on Windows, we'll immediately hit the current limitations.
So, without gdb changes, breakpoints in Eclipse are half-broken. In
fact, breakpoints in any frontend are half broken on Windows.

> My records seem to indicate 
> that a thread Re "fullname attribute for GDB/MI stack frames" in May
> 2005 on this list is one of them, but maybe there were more.  You will
> find my critique of what lrealpath does on Windows in a message in
> that thread I sent on May 29, and suggested ways to improve it in a
> followup message on the same day.  I think if we are going to use
> lrealpath, we should at least make its behavior consistent and correct
> on all supported platforms, including native Windows (i.e. MinGW).

Speaking of the issues you've raised in:

	http://sourceware.org/ml/gdb-patches/2005-05/msg00612.html

I think that:

1. The order of slashes is a cosmetic issue.
2. The case of filenames is also a cosmetic issues.
3. The matter of filename existance is a behaviour issue, and I think
I can modify gdb_realpath to perform a check explicitly. OTOH, it's not
clear if any code actually expects file existane check to be performed.

- Volodya

Re: Better realpath

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 14 Jun 2008 16:14:07 +0400
> Cc: gdb-patches@sources.redhat.com
> 
> Speaking of the issues you've raised in:
> 
> 	http://sourceware.org/ml/gdb-patches/2005-05/msg00612.html
> 
> I think that:
> 
> 1. The order of slashes is a cosmetic issue.
> 2. The case of filenames is also a cosmetic issues.

Would it surprise you that I disagree?

Fixing those is not a big deal, so how about making gdb_realpath
correct both cosmetically and behavior-wise?

> 3. The matter of filename existance is a behaviour issue, and I think
> I can modify gdb_realpath to perform a check explicitly. OTOH, it's not
> clear if any code actually expects file existane check to be performed.

I don't think it matters whether the callers expect it or not.  As
long as we use realpath, which always checks the result for existence,
we should do the same in the other branches, so that the resulting GDB
function behaves consistently.  Alternatively, we could refrain from
using realpath, in which case we should consistently _not_ require
that the file exists.

Re: Better realpath

[Vladimir Prus]

Eli Zaretskii wrote:

>> From: Vladimir Prus <vladimir@codesourcery.com>
>> Date: Sat, 14 Jun 2008 16:14:07 +0400
>> Cc: gdb-patches@sources.redhat.com
>> 
>> Speaking of the issues you've raised in:
>> 
>> http://sourceware.org/ml/gdb-patches/2005-05/msg00612.html
>> 
>> I think that:
>> 
>> 1. The order of slashes is a cosmetic issue.
>> 2. The case of filenames is also a cosmetic issues.
> 
> Would it surprise you that I disagree?
> 
> Fixing those is not a big deal, so how about making gdb_realpath
> correct both cosmetically and behavior-wise?

I'm not sure how big deal that is, but it appears we have a major
functionality bug for 3 years already, at least. I'm interested in
fixing that bug, and if somebody (for example you) find those cosmetic
changes important, I think they can be address by follow-up patches.

>> 3. The matter of filename existance is a behaviour issue, and I think
>> I can modify gdb_realpath to perform a check explicitly. OTOH, it's not
>> clear if any code actually expects file existane check to be performed.
> 
> I don't think it matters whether the callers expect it or not.  As
> long as we use realpath, which always checks the result for existence,
> we should do the same in the other branches, so that the resulting GDB
> function behaves consistently.  

If no caller of that function cares about this aspect of behaviour, why
should we bother about consistency. But anyway...

> Alternatively, we could refrain from 
> using realpath, in which case we should consistently _not_ require
> that the file exists.

... as I've said, I can modify gdb_realpath to check for file existance,
on Windows, which will make the behaviour of gdb_realpath the same
everywhere. 

- Volodya

Re: Better realpath

[Eli Zaretskii]

> From:  Vladimir Prus <vladimir@codesourcery.com>
> Date:  Sun, 15 Jun 2008 00:03:26 +0400
> 
> > Fixing those is not a big deal, so how about making gdb_realpath
> > correct both cosmetically and behavior-wise?
> 
> I'm not sure how big deal that is, but it appears we have a major
> functionality bug for 3 years already, at least. I'm interested in
> fixing that bug, and if somebody (for example you) find those cosmetic
> changes important, I think they can be address by follow-up patches.

That can be said about every patch submitted here.  We still request
that contributors do a clean job, instead of allowing their patches to
go in, and then fixing their job by follow-up patches.  I'm saying
let's do a clean job in this case.

> >> 3. The matter of filename existance is a behaviour issue, and I think
> >> I can modify gdb_realpath to perform a check explicitly. OTOH, it's not
> >> clear if any code actually expects file existane check to be performed.
> > 
> > I don't think it matters whether the callers expect it or not.  As
> > long as we use realpath, which always checks the result for existence,
> > we should do the same in the other branches, so that the resulting GDB
> > function behaves consistently.  
> 
> If no caller of that function cares about this aspect of behaviour, why
> should we bother about consistency.

Because gdb_realpath is an infrastructure function, so we should care
about future, as yet non-existent, callers, not only about the
existing ones.

> > Alternatively, we could refrain from 
> > using realpath, in which case we should consistently _not_ require
> > that the file exists.
> 
> ... as I've said, I can modify gdb_realpath to check for file existance,
> on Windows, which will make the behaviour of gdb_realpath the same
> everywhere. 

The question is, what kind of consistent behavior do we want.
Personally, I think realpath's behavior is wrong for most GDB usages,
that's why I suggested the alternative.

Re: Better realpath

[Daniel Jacobowitz]

On Sun, Jun 15, 2008 at 06:18:45AM +0300, Eli Zaretskii wrote:
> > > Alternatively, we could refrain from 
> > > using realpath, in which case we should consistently _not_ require
> > > that the file exists.
> > 
> > ... as I've said, I can modify gdb_realpath to check for file existance,
> > on Windows, which will make the behaviour of gdb_realpath the same
> > everywhere. 
> 
> The question is, what kind of consistent behavior do we want.
> Personally, I think realpath's behavior is wrong for most GDB usages,
> that's why I suggested the alternative.

It seems to me that the thing to do is eliminate gdb_realpath in favor
of lrealpath.  That's supposed to be a portable version of realpath,
so realpath semantics seem like the way to go.

Why do you think realpath's behavior is wrong for GDB?  The only times
we're interested in realpath should be when we have an actual on-disk
file.  If it doesn't exist, its expanded path is not important.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Better realpath

[Eli Zaretskii]

> Date: Sat, 14 Jun 2008 23:26:54 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Vladimir Prus <vladimir@codesourcery.com>,
> 	gdb-patches@sources.redhat.com
> 
> It seems to me that the thing to do is eliminate gdb_realpath in favor
> of lrealpath.  That's supposed to be a portable version of realpath,
> so realpath semantics seem like the way to go.

I don't mind doing so, although libiberty has other customers, which
could make it harder for us to do what we think is right (if it
happens to be different from what lrealpath does now).  Note that
right now, lrealpath does not behave consistently with realpath (if
the latter is unavailable), so it cannot be regarded as a portable
version of realpath, at least not entirely so.

> Why do you think realpath's behavior is wrong for GDB?

Not just for GDB, in general as well: it doesn't seem right to me to
expand a file name and check for its existence in the same primitive,
not to mention refuse to produce an expansion if the file does not
exist.  These are two separate tests, so they should be kept separate.
(I actually suspect that realpath was used because it's more
convenient -- no messy memory allocation issues -- but I have no facts
to back this up.)

Re: Better realpath

[Daniel Jacobowitz]

On Sun, Jun 15, 2008 at 08:58:29PM +0300, Eli Zaretskii wrote:
> I don't mind doing so, although libiberty has other customers, which
> could make it harder for us to do what we think is right (if it
> happens to be different from what lrealpath does now).  Note that
> right now, lrealpath does not behave consistently with realpath (if
> the latter is unavailable), so it cannot be regarded as a portable
> version of realpath, at least not entirely so.

I assume that it is simply an oversight when Windows support was
added, not any deliberate divergence, and the libiberty maintainers
would be receptive to improvement.

> Not just for GDB, in general as well: it doesn't seem right to me to
> expand a file name and check for its existence in the same primitive,
> not to mention refuse to produce an expansion if the file does not
> exist.  These are two separate tests, so they should be kept separate.
> (I actually suspect that realpath was used because it's more
> convenient -- no messy memory allocation issues -- but I have no facts
> to back this up.)

On the other hand, it seems perfectly reasonable to me; we can't know
where the file would really be if it existed, unless it exists, since
we don't know whether it would be a symlink.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Better realpath

[Eli Zaretskii]

> Date: Sun, 15 Jun 2008 21:29:33 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: vladimir@codesourcery.com, gdb-patches@sources.redhat.com
> 
> On Sun, Jun 15, 2008 at 08:58:29PM +0300, Eli Zaretskii wrote:
> > I don't mind doing so, although libiberty has other customers, which
> > could make it harder for us to do what we think is right (if it
> > happens to be different from what lrealpath does now).  Note that
> > right now, lrealpath does not behave consistently with realpath (if
> > the latter is unavailable), so it cannot be regarded as a portable
> > version of realpath, at least not entirely so.
> 
> I assume that it is simply an oversight when Windows support was
> added

But canonicalize_filename also behaves like the Windows version does.

> > Not just for GDB, in general as well: it doesn't seem right to me to
> > expand a file name and check for its existence in the same primitive,
> > not to mention refuse to produce an expansion if the file does not
> > exist.  These are two separate tests, so they should be kept separate.
> > (I actually suspect that realpath was used because it's more
> > convenient -- no messy memory allocation issues -- but I have no facts
> > to back this up.)
> 
> On the other hand, it seems perfectly reasonable to me; we can't know
> where the file would really be if it existed, unless it exists, since
> we don't know whether it would be a symlink.

I don't see any problem with this behavior for non-existing files.
And, if we want to resolve symlinks, the Windows code should also do
that.

Re: Better realpath

[Vladimir Prus]

On Sunday 15 June 2008 07:18:45 Eli Zaretskii wrote:
> > From:  Vladimir Prus <vladimir@codesourcery.com>
> > Date:  Sun, 15 Jun 2008 00:03:26 +0400
> > 
> > > Fixing those is not a big deal, so how about making gdb_realpath
> > > correct both cosmetically and behavior-wise?
> > 
> > I'm not sure how big deal that is, but it appears we have a major
> > functionality bug for 3 years already, at least. I'm interested in
> > fixing that bug, and if somebody (for example you) find those cosmetic
> > changes important, I think they can be address by follow-up patches.
> 
> That can be said about every patch submitted here.  We still request
> that contributors do a clean job, instead of allowing their patches to
> go in, and then fixing their job by follow-up patches.  I'm saying
> let's do a clean job in this case.

The problem in this case is that:
- you appear to be the only one who cares about this cosmetics, *and*
- this issues is unfixed for 3 years,

Which leads to me believe that those cosmetic issues are of bikeshed nature,
with no concensus likely in next 3 years. And keeping GDB with a major bug
does not seem like a good idea.

My opinion here is that:

   - The bug must be fixed,
   - Having gdb_realpath copy-paste bunch of code from libibery's lrealpath
   is not acceptable. 

Note that if we worry that changes we propose will break other libibery's clients,
we can add another function, say lrealpath2, that does what we need, refactor
guts of current lrealpath into an internal function, and make both lrealpath
and lrealpath2 call that internal function.

The first question is about checking for file existance. Assuming we don't want
this check, we basically get to rewrite gdb_realpath from scratch, making it
operate on a purely syntactic basis. Such a rewrite is probably not very hard,
and I have open-source code for that from another project -- though license issues
can be messy. I'm not sure if lack of symlink resolution will be an issue. Assuming
we want file existance checking, we'd just have to make Windows code do the check.

Second is down-casing. If we don't want brute down-casing, and we want truly canonic
names of paths, then "C:/documents and settings" should become "C:/Documents and Settings",
and that requires actually poking at the file system to see what exact spelling is stored. 
And that requires that all path components exist. My reading of msdn suggests that 
GetFullPathName does not do that, and I don't know if any other function does that. 
I have an implementation in terms of FindFirstFile that should compile on Win95 and 
later and do the right thing, but it's tricky, and again, there are license issues. 
We can also just skip downcasing, and rely on gdb file comparison macros to handle
that.

Finally, there's slash direction. I think this is non-issue -- we can either make
GDB use "\" everywhere, or normalize to "/", either in lrealpath, or elsewhere.

So, the approaches are:

1. Make lrealpath always check for file existance, and:
	- Either revise window case to get spelling from the filesystem, or
	- Add a flag "I don't care about case differences, don't downcase"

2. Write another function that does purely syntactic normalization of paths.
It will not change case of paths on windows, naturally.

Which of those approaches you:

- Will be willing to accept?
- Will be willing to hack on?

Thanks,
Volodya

Re: Better realpath

[Stan Shebs]

Eli Zaretskii wrote:
>> Date: Sat, 14 Jun 2008 23:26:54 -0400
>> From: Daniel Jacobowitz <drow@false.org>
>> Cc: Vladimir Prus <vladimir@codesourcery.com>,
>> 	gdb-patches@sources.redhat.com
>>
>> It seems to me that the thing to do is eliminate gdb_realpath in favor
>> of lrealpath.  That's supposed to be a portable version of realpath,
>> so realpath semantics seem like the way to go.
>>     
>
> I don't mind doing so, although libiberty has other customers, which
> could make it harder for us to do what we think is right (if it
> happens to be different from what lrealpath does now).  Note that
> right now, lrealpath does not behave consistently with realpath (if
> the latter is unavailable), so it cannot be regarded as a portable
> version of realpath, at least not entirely so.
>   
Most likely libiberty's other customers (read: other GNU components :-) 
) want consistent behavior, and I'm inclined to think they would prefer 
consistency with each other over consistency with "real" realpath. There 
is nothing preventing us from adding additional functions to libiberty 
also, if we want to split functionality in a better way. One maintenance 
bonus of libiberty usage that it forestalls the introduction of subtle 
dependencies on GDB internals, always a hazard with our tangle of headers.

Stan

Re: Better realpath

[DJ Delorie]

> Most likely libiberty's other customers (read: other GNU components
> :-) ) want consistent behavior, and I'm inclined to think they would
> prefer consistency with each other over consistency with "real"
> realpath.

The goal was consistency across platforms.  Not all platforms *have* a
realpath() function.

Re: Better realpath

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Wed, 18 Jun 2008 11:33:11 +0400
> Cc: gdb-patches@sources.redhat.com
> 
> The problem in this case is that:
> - you appear to be the only one who cares about this cosmetics

Why is that a ``problem''?  I thought that objections from any global
maintainer are sufficient to justify changing submitted code so as to
eliminate those objections.

Btw, I don't think I heard strong counter-opinions, either.

>  *and*
> - this issues is unfixed for 3 years,

I'm all for fixing them.

> Which leads to me believe that those cosmetic issues are of bikeshed nature,
> with no concensus likely in next 3 years.

Let's try not to be alarmed by the shadow of a dwarf.  I don't see any
reason yet to think a consensus cannot be reached on these issues.
The old discussions from 3 years ago were about a different issue, and
only touched lrealpath tangentially, wrt the "d:foo" case.  Also, the
focus was the DJGPP port of GDB.  Neither is an issue this time, so
I'd hope we can resolve this fairly quickly.

> And keeping GDB with a major bug does not seem like a good idea.

Agreed.

> My opinion here is that:
> 
>    - The bug must be fixed,
>    - Having gdb_realpath copy-paste bunch of code from libibery's lrealpath
>    is not acceptable. 

I already said that I have no objections to modifying lrealpath.

> The first question is about checking for file existance. Assuming we don't want
> this check, we basically get to rewrite gdb_realpath from scratch, making it
> operate on a purely syntactic basis.

I don't think such radical measures would be necessary.  We could
either (a) use canonicalize_filename, which doesn't check for
existence, or (2) use realpath on the argument's leading directories
(i.e. call `dirname' to remove the last portion of the file name).  Am
I missing something?

> Second is down-casing. If we don't want brute down-casing, and we want truly canonic
> names of paths, then "C:/documents and settings" should become "C:/Documents and Settings",
> and that requires actually poking at the file system to see what exact spelling is stored. 

No, that's not necessary either.  All you need is run the result of
GetFullPathName through GetLongPathName: if it fails, it means the
file does not exist, and you need to return it in whatever letter-case
it was passed to us; if it succeeds, it will return the file name as
it's recorded in the filesystem.  For example, calling GetLongPathName
with either "C:/documents and settings" or "C:/DOCUME~1" will return
"C:\Documents and Settings".

> My reading of msdn suggests that GetFullPathName does not do that

Right, it doesn't.

> Finally, there's slash direction. I think this is non-issue -- we can either make
> GDB use "\" everywhere, or normalize to "/", either in lrealpath, or elsewhere.

I'd prefer forward slashes.

> So, the approaches are:
> 
> 1. Make lrealpath always check for file existance, and:
> 	- Either revise window case to get spelling from the filesystem, or
> 	- Add a flag "I don't care about case differences, don't downcase"
> 
> 2. Write another function that does purely syntactic normalization of paths.
> It will not change case of paths on windows, naturally.
> 
> Which of those approaches you:
> 
> - Will be willing to accept?
> - Will be willing to hack on?

I hope I answered those questions now.  If not, please tell.

Thanks.

Re: Better realpath

[Vladimir Prus]

On Wednesday 18 June 2008 22:54:13 Eli Zaretskii wrote:

> > The first question is about checking for file existance. Assuming we don't want
> > this check, we basically get to rewrite gdb_realpath from scratch, making it
> > operate on a purely syntactic basis.
> 
> I don't think such radical measures would be necessary.  We could
> either (a) use canonicalize_filename, which doesn't check for
> existence, 

Hmm, the documentation at

	http://www.gnu.org/software/libc/manual/html_mono/libc.html

say:

    Function: char * canonicalize_file_name (const char *name)
	
	If any of the path components is missing the function returns a NULL pointer. 
    ....
    Function: char * realpath (const char *restrict name, char *restrict resolved)
 
	A call to realpath where the resolved parameter is NULL behaves exactly like 
	canonicalize_file_name. The function allocates a buffer for the file name and 
	returns a pointer to it. If resolved is not NULL it points to a buffer into 
	which the result is copied. It is the callers responsibility to allocate a 
	buffer which is large enough. On systems which define PATH_MAX this means 
	the buffer must be large enough for a pathname of this size. For systems
	without limitations on the pathname length the requirement cannot be met
	and programs should not call realpath with anything but NULL for the 
	second parameter. 

	One other difference is that the buffer resolved (if nonzero) will contain the 
	part of the path component which does not exist or is not readable if the 
	function returns NULL and errno is set to EACCES or ENOENT. 

From that, I don't quite understand why you think canonicalize_file_name does not
check for file existance. Is documentation in error?

> or (2) use realpath on the argument's leading directories 
> (i.e. call `dirname' to remove the last portion of the file name).  Am
> I missing something?

And this will check dirname existance? This semantics is mid-way between checking
everything for existance, and not checking anything. Is this really intuitive and
desirable?

> > Second is down-casing. If we don't want brute down-casing, and we want truly canonic
> > names of paths, then "C:/documents and settings" should become "C:/Documents and Settings",
> > and that requires actually poking at the file system to see what exact spelling is stored. 
> 
> No, that's not necessary either.  All you need is run the result of
> GetFullPathName through GetLongPathName: if it fails, it means the
> file does not exist, and you need to return it in whatever letter-case
> it was passed to us; if it succeeds, it will return the file name as
> it's recorded in the filesystem.  

That does not contradict what I say -- it *does* require poking at the file system,
so if some component of path is missing, you get no canonical representation.
The approach of returning paths that don't exist unmodified seem risky, in particular...

> For example, calling GetLongPathName 
> with either "C:/documents and settings" or "C:/DOCUME~1" will return
> "C:\Documents and Settings".

... what will be the return value of GetLongPathName on "C:/DOCUME~1/nonexistent/nonexistent2"
and "C:/documents and settings/nonexistent/nonexistent2". Presumably, GetLongPathName will
fail in both cases, and GDB will think those paths are unequal.

GetLongPathName, also, is not available on Windows 95. Is that an issue?

> > So, the approaches are:
> > 
> > 1. Make lrealpath always check for file existance, and:
> > 	- Either revise window case to get spelling from the filesystem, or
> > 	- Add a flag "I don't care about case differences, don't downcase"
> > 
> > 2. Write another function that does purely syntactic normalization of paths.
> > It will not change case of paths on windows, naturally.
> > 
> > Which of those approaches you:
> > 
> > - Will be willing to accept?
> > - Will be willing to hack on?
> 
> I hope I answered those questions now.  If not, please tell.

No, I don't think I have found clear answers to either of those questions. You comments
suggested another approach, so here's the updated list of alternatives:

1. Make lrealpath always check for file existance. On Windows, make it use
GetLongPathName, instead of lowercasing, to get canonical path name.

2. Make lrealpath check for dirname existance only. The filename part will have to
be downcased on Windows.

3. Implement syntax-only simplification function; it won't be able to use the exact
spelling as stored in filesystem on Windows, and it won't be able to handle short
names.

It appears to me that (1) is the easiest approach, minimally disturbing for
existing code, and the one that I personally can implement and defend before libibery
maintainers. Now, which of those approaches you:

 - Will be willing to accept?
 - Will be willing to hack on, and push in libibery?

- Volodya

Re: Better realpath

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Thu, 19 Jun 2008 10:06:42 +0400
> Cc: gdb-patches@sources.redhat.com
> 
> > I don't think such radical measures would be necessary.  We could
> > either (a) use canonicalize_filename, which doesn't check for
> > existence, 
> 
> Hmm, the documentation at
> 
> 	http://www.gnu.org/software/libc/manual/html_mono/libc.html
> 
> say:
> 
>     Function: char * canonicalize_file_name (const char *name)
> 	
> 	If any of the path components is missing the function returns a NULL pointer. 

My version of libc.info says something different:

     In any of the path components except the last one is missing the
     function returns a NULL pointer.

The "except the last" part makes all the difference.

I guess we need to try this to see which one is true.

> > or (2) use realpath on the argument's leading directories 
> > (i.e. call `dirname' to remove the last portion of the file name).  Am
> > I missing something?
> 
> And this will check dirname existance? This semantics is mid-way between checking
> everything for existance, and not checking anything. Is this really intuitive and
> desirable?

I was aiming for consistency with canonicalize_file_name, assuming it
behaves like my reference above says.

> > > Second is down-casing. If we don't want brute down-casing, and we want truly canonic
> > > names of paths, then "C:/documents and settings" should become "C:/Documents and Settings",
> > > and that requires actually poking at the file system to see what exact spelling is stored. 
> > 
> > No, that's not necessary either.  All you need is run the result of
> > GetFullPathName through GetLongPathName: if it fails, it means the
> > file does not exist, and you need to return it in whatever letter-case
> > it was passed to us; if it succeeds, it will return the file name as
> > it's recorded in the filesystem.  
> 
> That does not contradict what I say -- it *does* require poking at the file system,

Well, _any_ file-related system call eventually pokes the file system.
What I meant is that we don't need to do that yourself in application
code.

> ... what will be the return value of GetLongPathName on "C:/DOCUME~1/nonexistent/nonexistent2"
> and "C:/documents and settings/nonexistent/nonexistent2". Presumably, GetLongPathName will
> fail in both cases, and GDB will think those paths are unequal.

That's true, but it's no worse than today: FILENAME_CMP does not
handle comparison of short 8+3 aliases with their long variants.  So
we don't need to do this if we don't want to.  If we do want to handle
these cases, I guess we could iteratively chop directories from the
right and run them through GetLongPathName until we get to one that
does exist.  But I don't know if this is worth the hassle.  After all,
in the context of GDB, a file name recorded in the debug info could
well cease to exist by the time GDB tries to canonicalize its name.

> GetLongPathName, also, is not available on Windows 95. Is that an issue?

I don't think so.

> 2. Make lrealpath check for dirname existance only. The filename part will have to
> be downcased on Windows.

It doesn't need to be downcased: FILENAME_CMP copes with letter-case
differences.

> Now, which of those approaches you:
> 
>  - Will be willing to accept?

Any one of them, actually.

>  - Will be willing to hack on, and push in libibery?

"Willing", yes; "have time to", probably no.