Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Eli Zaretskii" <eliz@is.elta.co.il>
To: brobecker@ACT-Europe.FR
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFC] xfullpath and new regression test xfullpath.exp
Date: Wed, 03 Apr 2002 05:12:00 -0000	[thread overview]
Message-ID: <7458-Wed03Apr2002160810+0300-eliz@is.elta.co.il> (raw)
In-Reply-To: <20020402172232.C25687@act-europe.fr> (message from Joel Brobecker on Tue, 2 Apr 2002 17:22:33 +0200)

> Is the following style more in line with the GNU project style?
> 
>   /* Return a copy of FILENAME, with its directory prefix canonicalized
>      by gdb_realpath.  */

Yes.

> Ah, Ok, I did not know this, I thought that d:foo and d:/foo were
> equivalent. Shouldn't xfullpath return d:<cwd>/foo instead? What do you
> think of the following pseudo-code?
> 
>    [...]
>    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';
> 
>    #if defined(__CYGWIN__) || defined(__MINGW32__)
>    if (strlen (dir_name) == 2 &&
>        is_letter (dir_name[0]) && dir_name[1] == ':')
>      {
>        dir_name[2] = '.';
>        dir_name[3] = '\000';
>      }
>    #endif
> 
>    real_path = gdb_realpath (dir_name);

Yes, this is okay, but please don't use system-dependent symbols like
__CYGWIN__ etc. if you can avoid that: we want to avoid
system-dependent preprocessor symbols as much as we can.  In this
case, include/filename.h already defines a non-zero value for the
macro HAVE_DOS_BASED_FILE_SYSTEM on systems that need that special
code.  So please use HAVE_DOS_BASED_FILE_SYSTEM instead.

> Is this better if I modify the last part of the function to be
> 
>    if (IS_DIR_SEPARATOR (real_path[strlen (real_path) - 1]))
>      result = concat (real_path, base_name, NULL);
>    else
>      result = concat (real_path, SLASH_STRING, base_name, NULL);

Yes, I think this is better, thanks.

> (can I consider that SLASH_STRING will always be one character long?):

Where does the code assume it's a single character?  `concat' conses
up a new string, right?  If so, the length of SLASH_STRING shouldn't
matter, I think.  Or am I missing something?


  reply	other threads:[~2002-04-03 13:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-04-01  0:50 Joel Brobecker
2002-04-01  1:43 ` Eli Zaretskii
2002-04-02  7:22   ` Joel Brobecker
2002-04-03  5:12     ` Eli Zaretskii [this message]
2002-04-03  5:19       ` Joel Brobecker
2002-04-04 23:44         ` Eli Zaretskii
2002-04-03 20:43 ` Andrew Cagney

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=7458-Wed03Apr2002160810+0300-eliz@is.elta.co.il \
    --to=eliz@is.elta.co.il \
    --cc=brobecker@ACT-Europe.FR \
    --cc=gdb-patches@sources.redhat.com \
    /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