Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: gdb-patches@sourceware.org, Christopher Faylor <me@cgf.cx>
Subject: Re: RFA: Use lrealpath instead of gdb_realpath
Date: Sun, 29 May 2005 18:22:00 -0000	[thread overview]
Message-ID: <umzqd9a5m.fsf@gnu.org> (raw)
In-Reply-To: <20050529142112.GB23858@nevyn.them.org> (message from Daniel Jacobowitz on Sun, 29 May 2005 10:21:12 -0400)

> Date: Sun, 29 May 2005 10:21:12 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: gdb-patches@sourceware.org, Christopher Faylor <me@cgf.cx>
> 
> >  . 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 maintainers disagree with you.  So, for the record, do I.

Disagree with what part? that backslashes might cause trouble or that
D:\foo\bar/baz.c is ugly?

> >  . 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.
> 
> These were both implemented by Danny Smith.  You were furious with me
> for daring to disagree with you on how DJGPP handles filenames; and now
> you're critiquing the mingw maintainers for how they choose to handle
> filenames?  Please, pardon me if I find that a bit ironic.

This has nothing to do with the previous argument: I accept its
outcome as ``fait accompli''.  I just thought you wanted to hear about
some of the subtle points in using the current code in lrealpath.  I
don't think I talked about these issues in the previous thread, at
least not about all of them.

Yes, this is a critique on how MinGW maintainers handle file names.  I
assume that those maintainers want to hear suggestions for improving
their code, and that we, the GDB maintainers, want GDB to DTRT on
Windows as it does on Posix platforms.  If the MinGW people decide to
disregard or become furious, that is their prerogative; but I still
would like to speak up in the hope that they will find my critique
useful and constructive.

> It is not worth my time to continue arguing about filename handling;
> consider the patch withdrawn indefinitely.

My suggestion is to fix lrealpath so that it (a) converts all
backslashes to forward slashes, (b) does NOT downcase the result, and
(c) returns its argument unaltered if the absolute file name does not
point to an existing file.  When modified like that, I think lrealpath
will work the same on all platforms; as for strdup vs xstrdup part, we
will need to change the code in GDB to test for a NULL return value.

If this alternative is acceptable to other maintainers, I can
volunteer to produce a patch along these lines.


  parent reply	other threads:[~2005-05-29 18:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-28 23:59 Daniel Jacobowitz
2005-05-29  1:59 ` Christopher Faylor
2005-05-29 14:21 ` Eli Zaretskii
2005-05-29 14:23   ` Daniel Jacobowitz
2005-05-29 17:49     ` Christopher Faylor
2005-05-29 18:22     ` Eli Zaretskii [this message]
2005-05-29 18:27       ` Christopher Faylor
2005-05-30 16:54         ` Eli Zaretskii
2005-05-29 19:28       ` Daniel Jacobowitz
2005-05-29 20:16         ` Christopher Faylor
2005-05-30 15:14           ` Christopher Faylor
2005-05-30 18:32           ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=umzqd9a5m.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    --cc=me@cgf.cx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox