This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] xfullpath and new regression test xfullpath.exp


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]