This is the mail archive of the gdb-patches@sourceware.org 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: proposal: substitute-path handles foreign dir separators


> Date: Fri, 17 Dec 2010 15:18:13 +0100
> From: Raphael Zulliger <zulliger@indel.ch>
> 
>   1. Most importantly: Do we want such a 'feature'? Personally, I think 
> it makes sense to have such a feature.

There were discussions on this a few months ago, you may wish to look
them up.

FWIW, I'm not opposed to having this feature, but I think it should
not be on by default on Posix platforms, because they support file
names with literal backslashes.

>   2. I created this patch because I actually debug binaries on a Linux 
> box which have been created on a Windows machine (GNU toolchain/Cygwin). 
> Empirically, I've realized that 'substitute-path' doesn't work for this 
> purpose. After having single-stepped GDB and modified its code slightly, 
> I ended up with this patch. The point is that I figured it out 
> empirically. It could the case that I justed missed an already existing 
> solution. Please let me know if my patch is crap because there exists 
> another way of handling my use case.

I don't think your approach is ``crap'', but the patch needs work,
IMO.  See below.

> !   if (path[from_len] != '\0' && (!IS_UNIX_DIR_SEPARATOR (path[from_len]) && !IS_DOS_DIR_SEPARATOR (path[from_len])))

This isn't the right way, for 2 reasons:

 . It makes the code uglier than it must be.

 . The support for DOS-style file names is based on compile-time
   macros, so it cannot be turned on and off.  Thus, you leave no fire
   escape for Unix users who just happen to have file names with
   literal backslashes, improbable as that may be.

The first part could be taken care of by crafting a single macro that
replaces the two macros.  The second part calls for an option which
could be tested in the same macro.

> !   if (IS_UNIX_ABSOLUTE_PATH (filename) || IS_DOS_ABSOLUTE_PATH(filename))

Similarly here.

>         if (rewritten_filename != NULL)
>           {
> +     	  /* Especially for embedded systems, it may be the case that a
> +     	 binary has been built on Windows but the embedded system is now
> +     	 being debugged on a Unix machine (and vice versa). In order to
> +     	 make path substitution work on such 'mixed' path styles, we need
> +     	 to convert foreign dir separators to native ones. */
> + #ifdef HAVE_DOS_BASED_FILE_SYSTEM
> +     	  const char from = '/';
> +     	  const char to = '\\';
> + #else
> +     	  const char from = '\\';
> +     	  const char to = '/';
> + #endif
> +           unsigned int i=0;
> +           for( i=0; i<strlen(rewritten_filename); ++i ) {
> +         	  if( rewritten_filename[i] == from ) {
> +         		  rewritten_filename[i] = to;
> +         	  }
> +           }

In this part, I simply don't understand why you needed the
HAVE_DOS_BASED_FILE_SYSTEM branch.  DOS/Windows file-system calls
support forward slashes just fine, so there's no need to rewrite the
slashes.

Finally, please wait for other opinions, as I'm not sure mine is in
consensus.


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