Mirror of the gdb mailing list
 help / color / mirror / Atom feed
* Build question
@ 2009-08-21  0:24 Danny Backx
  2009-08-21 17:30 ` Tom Tromey
  0 siblings, 1 reply; 19+ messages in thread
From: Danny Backx @ 2009-08-21  0:24 UTC (permalink / raw)
  To: gdb

Advice wanted on how to patch gdb's build.

I have a cross-gdb working between my linux PC and a i386-mingw32ce
target running gdbserver.

My question is about the i386-mingw32ce-gdb build.
The issue I need to solve is that HAVE_DOS_BASED_FILE_SYSTEM doesn't get
defined. I can manipulate the build so things work, but I'd like to make
a clean patch.

HAVE_DOS_BASED_FILE_SYSTEM doesn't get set in my cross-compile, because
gdb gets compiled by the host gcc, which doesn't set _WIN32. So the
HAVE_DOS_BASED_FILE_SYSTEM never gets defined. This happens both in
gdb/* and in libiberty/* .

So where should I change this properly ?

I would like it to come from somewhere in the configuration scripts (and
I'm now asking where would be the best place).

Another thing to edit is then include/filenames.h - it needs to grab
this and do the right thing, instead of just reacting to _WIN32 and
such. But that's a trivial fix.

Thanks,

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-21  0:24 Build question Danny Backx
@ 2009-08-21 17:30 ` Tom Tromey
  2009-08-21 17:57   ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Tom Tromey @ 2009-08-21 17:30 UTC (permalink / raw)
  To: danny.backx; +Cc: gdb

>>>>> "Danny" == Danny Backx <danny.backx@scarlet.be> writes:

Danny> HAVE_DOS_BASED_FILE_SYSTEM doesn't get set in my cross-compile, because
Danny> gdb gets compiled by the host gcc, which doesn't set _WIN32. So the
Danny> HAVE_DOS_BASED_FILE_SYSTEM never gets defined. This happens both in
Danny> gdb/* and in libiberty/* .

Just to be certain I understand, you're building a gdb that runs on your
Linux box but that you use to debug something running on Windows?

Danny> So where should I change this properly ?

Danny> Another thing to edit is then include/filenames.h - it needs to grab
Danny> this and do the right thing, instead of just reacting to _WIN32 and
Danny> such. But that's a trivial fix.

I am not familiar with this area in depth.  From what you've said it
sounds like gdb has some confusion about host paths and target paths.
Assuming that is correct, then unfortunately for you I think the best
thing to do would be to separate these concepts and turn the current
HAVE_DOS_BASED_FILE_SYSTEM into a target property.

Tom


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-21 17:30 ` Tom Tromey
@ 2009-08-21 17:57   ` Eli Zaretskii
  2009-08-21 18:23     ` Danny Backx
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2009-08-21 17:57 UTC (permalink / raw)
  To: tromey; +Cc: danny.backx, gdb

> From: Tom Tromey <tromey@redhat.com>
> Cc: gdb@sourceware.org
> Date: Fri, 21 Aug 2009 10:56:28 -0600
> 
> I am not familiar with this area in depth.  From what you've said it
> sounds like gdb has some confusion about host paths and target paths.

The way these macros and the corresponding source fragments in GDB are
set, they only DTRT for native debugging.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-21 17:57   ` Eli Zaretskii
@ 2009-08-21 18:23     ` Danny Backx
  2009-08-21 18:51       ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Danny Backx @ 2009-08-21 18:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb

On Fri, 2009-08-21 at 10:56 -0600, Tom Tromey wrote:
> Just to be certain I understand, you're building a gdb that runs on
> your Linux box but that you use to debug something running on Windows?

Yes. Windows CE actually - PDA's, smartphones, etc.

gdbserver runs on them (required only a small tweak), that's what gdb on
Linux talks to.

On Fri, 2009-08-21 at 20:31 +0300, Eli Zaretskii wrote:
> > From: Tom Tromey <tromey@redhat.com>
> > I am not familiar with this area in depth.  From what you've said it
> > sounds like gdb has some confusion about host paths and target paths.
> 
> The way these macros and the corresponding source fragments in GDB are
> set, they only DTRT for native debugging.

Also right, but it looks like it doesn't take much to make it just work.

Tom wrote :
> Assuming that is correct, then unfortunately for you I think the best
> thing to do would be to separate these concepts and turn the current
> HAVE_DOS_BASED_FILE_SYSTEM into a target property.

Can you point me in the right direction ?

Thanks,

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-21 18:23     ` Danny Backx
@ 2009-08-21 18:51       ` Eli Zaretskii
  2009-08-21 20:15         ` Danny Backx
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2009-08-21 18:51 UTC (permalink / raw)
  To: danny.backx; +Cc: tromey, gdb

> From: Danny Backx <danny.backx@scarlet.be>
> Cc: tromey@redhat.com, gdb@sourceware.org
> Date: Fri, 21 Aug 2009 20:05:00 +0200
> 
> On Fri, 2009-08-21 at 20:31 +0300, Eli Zaretskii wrote:
> > > From: Tom Tromey <tromey@redhat.com>
> > > I am not familiar with this area in depth.  From what you've said it
> > > sounds like gdb has some confusion about host paths and target paths.
> > 
> > The way these macros and the corresponding source fragments in GDB are
> > set, they only DTRT for native debugging.
> 
> Also right, but it looks like it doesn't take much to make it just work.

I think they need to be converted to run-time tests instead.  For
that, we would probably need some kind of primitive that returns the
filesystem type of the target, what the macros do now at compile-time.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-21 18:51       ` Eli Zaretskii
@ 2009-08-21 20:15         ` Danny Backx
  2009-08-21 21:57           ` Eli Zaretskii
  2009-08-22  9:03           ` Tom Tromey
  0 siblings, 2 replies; 19+ messages in thread
From: Danny Backx @ 2009-08-21 20:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb

On Fri, 2009-08-21 at 21:21 +0300, Eli Zaretskii wrote: 
> > > The way these macros and the corresponding source fragments in GDB are
> > > set, they only DTRT for native debugging.
> > 
> > Also right, but it looks like it doesn't take much to make it just work.
> 
> I think they need to be converted to run-time tests instead.  For
> that, we would probably need some kind of primitive that returns the
> filesystem type of the target, what the macros do now at compile-time.

That doesn't look right.

I am building i386-mingw32ce-gdb (or arm-mingw32ce-gdb). The targets
specified are Windows CE based, so they all have this "DOS BASED" file
system. So why test this at run time ?

The total extent of the patches I needed to get this particular issue
out of the way is this :
pavilion: {868} cvs diff include/filenames.h
Index: include/filenames.h
===================================================================
RCS file: /cvs/src/src/include/filenames.h,v
retrieving revision 1.5
diff -u -r1.5 filenames.h
--- include/filenames.h 21 Mar 2008 23:40:18 -0000      1.5
+++ include/filenames.h 21 Aug 2009 18:41:44 -0000
@@ -36,6 +36,10 @@
 #define HAVE_DOS_BASED_FILE_SYSTEM 1
 #endif
 
+#endif
+
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+
 #define IS_DIR_SEPARATOR(c)    ((c) == '/' || (c) == '\\')
 /* Note that IS_ABSOLUTE_PATH accepts d:foo as well, although it is
    only semi-absolute.  This is because the users of IS_ABSOLUTE_PATH

And in the build directory, I added
 #define HAVE_DOS_BASED_FILE_SYSTEM 1
to libiberty/config.h and gdb/config.h .

I am looking for a way to get this into the build system somehow.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-21 20:15         ` Danny Backx
@ 2009-08-21 21:57           ` Eli Zaretskii
  2009-08-22  9:03           ` Tom Tromey
  1 sibling, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2009-08-21 21:57 UTC (permalink / raw)
  To: danny.backx; +Cc: tromey, gdb

> From: Danny Backx <danny.backx@scarlet.be>
> Cc: tromey@redhat.com, gdb@sourceware.org
> Date: Fri, 21 Aug 2009 20:52:26 +0200
> 
> On Fri, 2009-08-21 at 21:21 +0300, Eli Zaretskii wrote: 
> > > > The way these macros and the corresponding source fragments in GDB are
> > > > set, they only DTRT for native debugging.
> > > 
> > > Also right, but it looks like it doesn't take much to make it just work.
> > 
> > I think they need to be converted to run-time tests instead.  For
> > that, we would probably need some kind of primitive that returns the
> > filesystem type of the target, what the macros do now at compile-time.
> 
> That doesn't look right.
> 
> I am building i386-mingw32ce-gdb (or arm-mingw32ce-gdb). The targets
> specified are Windows CE based, so they all have this "DOS BASED" file
> system. So why test this at run time ?

I thought the intent was to make it possible to have targets that use
``DOS-based'' filesystems and targets that use Posix filesystems in
the same session.  GDB does support different targets in the same
session, right?

If you want a version that targets only one, then yes, some suitable
change in the macros will do what you want.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-21 20:15         ` Danny Backx
  2009-08-21 21:57           ` Eli Zaretskii
@ 2009-08-22  9:03           ` Tom Tromey
  2009-08-22  9:55             ` Danny Backx
  2009-08-22 22:01             ` Daniel Jacobowitz
  1 sibling, 2 replies; 19+ messages in thread
From: Tom Tromey @ 2009-08-22  9:03 UTC (permalink / raw)
  To: danny.backx; +Cc: Eli Zaretskii, gdb

>>>>> "Danny" == Danny Backx <danny.backx@scarlet.be> writes:

Danny> I am building i386-mingw32ce-gdb (or arm-mingw32ce-gdb). The targets
Danny> specified are Windows CE based, so they all have this "DOS BASED" file
Danny> system. So why test this at run time ?

IIUC, the problem is that gdb doesn't really differentiate between
target file names and host file names.  This matters because some things
are searched for on the host but some on the target.

If you want to always define HAVE_DOS_BASED_FILE_SYSTEM, I guess that is
possible.  It is hard to picture when that would be appropriate for CVS
GDB, though, due to --enable-targets=all.

If you don't want to mix different kinds of file names, then could you
further explain this note?

http://sourceware.org/ml/gdb-patches/2009-08/msg00160.html

It looks to me that the "good" scenario here mixes Unix-like and
DOS-like filenames, but I don't understand that.

Tom


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-22  9:03           ` Tom Tromey
@ 2009-08-22  9:55             ` Danny Backx
  2009-08-22 11:07               ` Eli Zaretskii
  2009-08-22 22:01             ` Daniel Jacobowitz
  1 sibling, 1 reply; 19+ messages in thread
From: Danny Backx @ 2009-08-22  9:55 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Eli Zaretskii, gdb

On Fri, 2009-08-21 at 15:56 -0600, Tom Tromey wrote:
> >>>>> "Danny" == Danny Backx <danny.backx@scarlet.be> writes:
> 
> Danny> I am building i386-mingw32ce-gdb (or arm-mingw32ce-gdb). The targets
> Danny> specified are Windows CE based, so they all have this "DOS BASED" file
> Danny> system. So why test this at run time ?
> 
> IIUC, the problem is that gdb doesn't really differentiate between
> target file names and host file names.  This matters because some things
> are searched for on the host but some on the target.
> 
> If you want to always define HAVE_DOS_BASED_FILE_SYSTEM, I guess that is
> possible.  It is hard to picture when that would be appropriate for CVS
> GDB, though, due to --enable-targets=all.
> 
> If you don't want to mix different kinds of file names, then could you
> further explain this note?
> 
> http://sourceware.org/ml/gdb-patches/2009-08/msg00160.html
> 
> It looks to me that the "good" scenario here mixes Unix-like and
> DOS-like filenames, but I don't understand that.

Looks like I didn't think this through completely.

Yes, the "good" scenario does mix things source and target file names (I
guess the "info share" will always do that). If the file names have
different syntax, then this is potentially hard.

Without going too deep : without changes, gdbserver reports Windows file
names. The gdb will try to find these locally (mixup 1), then if it
fails, try to separate path from file name (mixup 2), and then try to
find the file name locally again.

In a cross-debugging case, mixup 1 is almost certain to fail. That
doesn't bother me much. The other issue is, though. I've shown the issue
with "info share" but obviously the impact goes further than that : if
gdb can't find the DLL locally then its debugging capabilities will be
crippled.

I can think of several ways out of this.
1. The one I suggested earlier. May not be the right solution.
2. Generalize target filename handling in gdb. Might be much harder than
   expected.
3. Separate directory and file name in the communication between
   gdbserver and gdb, so mixup 2 is avoided.
4. Extend gdbserver/gdb communication so the target file name syntax
   is reported back.
5. Do (4) but in gdb target properties (this may be what Eli said).
   Would require extending the gdb target definitions.

Comments ?

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-22  9:55             ` Danny Backx
@ 2009-08-22 11:07               ` Eli Zaretskii
  2009-08-24 10:11                 ` Danny Backx
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2009-08-22 11:07 UTC (permalink / raw)
  To: danny.backx; +Cc: tromey, gdb

> From: Danny Backx <danny.backx@scarlet.be>
> Cc: Eli Zaretskii <eliz@gnu.org>, gdb@sourceware.org
> Date: Sat, 22 Aug 2009 11:04:58 +0200
> 
> 1. The one I suggested earlier. May not be the right solution.
> 2. Generalize target filename handling in gdb. Might be much harder than
>    expected.
> 3. Separate directory and file name in the communication between
>    gdbserver and gdb, so mixup 2 is avoided.
> 4. Extend gdbserver/gdb communication so the target file name syntax
>    is reported back.
> 5. Do (4) but in gdb target properties (this may be what Eli said).
>    Would require extending the gdb target definitions.
> 
> Comments ?

Can you point out the places in the code (hopefully, not too many)
where the Posix assumption about file-name syntax needs to be replaced
with the Windows assumption?

If there are not too many of them, we could modify them to use one or
two user options.  For starters, these options would need to be set
manually, by the user who knows what filesystem she is working with.
Later, we could try to set them automatically.

WDYT?


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-22  9:03           ` Tom Tromey
  2009-08-22  9:55             ` Danny Backx
@ 2009-08-22 22:01             ` Daniel Jacobowitz
  2009-08-23  7:28               ` Joel Brobecker
  1 sibling, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2009-08-22 22:01 UTC (permalink / raw)
  To: Tom Tromey; +Cc: danny.backx, Eli Zaretskii, gdb

On Fri, Aug 21, 2009 at 03:56:45PM -0600, Tom Tromey wrote:
> IIUC, the problem is that gdb doesn't really differentiate between
> target file names and host file names.  This matters because some things
> are searched for on the host but some on the target.
> 
> If you want to always define HAVE_DOS_BASED_FILE_SYSTEM, I guess that is
> possible.  It is hard to picture when that would be appropriate for CVS
> GDB, though, due to --enable-targets=all.

Joel, didn't you have a patch for this at some point?  Or am I
imagining things?

I'm of the opinion that we could save ourselves a heck of a lot of
trouble by allowing both DOS and POSIX pathnames.  This breaks on
POSIX systems where you have directories starting with "c:" in the
current directory, or files containing backslashes - both quite
unlikely.  The only touchy bit is case sensitivity, of course...

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-22 22:01             ` Daniel Jacobowitz
@ 2009-08-23  7:28               ` Joel Brobecker
  0 siblings, 0 replies; 19+ messages in thread
From: Joel Brobecker @ 2009-08-23  7:28 UTC (permalink / raw)
  To: Tom Tromey, danny.backx, Eli Zaretskii, gdb

> > If you want to always define HAVE_DOS_BASED_FILE_SYSTEM, I guess that is
> > possible.  It is hard to picture when that would be appropriate for CVS
> > GDB, though, due to --enable-targets=all.
> 
> Joel, didn't you have a patch for this at some point?  Or am I
> imagining things?

Not that I remember. To me, I've always wanted the debugger to use
the filesystem conventions of the host (I haven't had a chance to
look at the case where it would make sense to use the filesystem
conventions of the target, yet).

-- 
Joel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-22 11:07               ` Eli Zaretskii
@ 2009-08-24 10:11                 ` Danny Backx
  2009-09-01 18:03                   ` Danny Backx
  0 siblings, 1 reply; 19+ messages in thread
From: Danny Backx @ 2009-08-24 10:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb

On Sat, 2009-08-22 at 12:52 +0300, Eli Zaretskii wrote:
> > From: Danny Backx <danny.backx@scarlet.be>
> > Cc: Eli Zaretskii <eliz@gnu.org>, gdb@sourceware.org
> > Date: Sat, 22 Aug 2009 11:04:58 +0200
> > 
> > 1. The one I suggested earlier. May not be the right solution.
> > 2. Generalize target filename handling in gdb. Might be much harder than
> >    expected.
> > 3. Separate directory and file name in the communication between
> >    gdbserver and gdb, so mixup 2 is avoided.
> > 4. Extend gdbserver/gdb communication so the target file name syntax
> >    is reported back.
> > 5. Do (4) but in gdb target properties (this may be what Eli said).
> >    Would require extending the gdb target definitions.
> > 
> > Comments ?
> 
> Can you point out the places in the code (hopefully, not too many)
> where the Posix assumption about file-name syntax needs to be replaced
> with the Windows assumption?
> 
> If there are not too many of them, we could modify them to use one or
> two user options.  For starters, these options would need to be set
> manually, by the user who knows what filesystem she is working with.
> Later, we could try to set them automatically.

I'll try this.

Daniel Jacobowitz wrote :
> I'm of the opinion that we could save ourselves a heck of a lot of
> trouble by allowing both DOS and POSIX pathnames.  This breaks on
> POSIX systems where you have directories starting with "c:" in the
> current directory, or files containing backslashes - both quite
> unlikely.  The only touchy bit is case sensitivity, of course...

This is the simpler assumption, that I was somehow wondering about but
didn't write. But if I do the above, then getting to this should be
easy.

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-08-24 10:11                 ` Danny Backx
@ 2009-09-01 18:03                   ` Danny Backx
  2009-09-01 19:34                     ` Eli Zaretskii
  2009-09-01 19:35                     ` Eli Zaretskii
  0 siblings, 2 replies; 19+ messages in thread
From: Danny Backx @ 2009-09-01 18:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb

On Mon, 2009-08-24 at 08:26 +0200, Danny Backx wrote:
> On Sat, 2009-08-22 at 12:52 +0300, Eli Zaretskii wrote:
> > Can you point out the places in the code (hopefully, not too many)
> > where the Posix assumption about file-name syntax needs to be replaced
> > with the Windows assumption?
> > 
> > If there are not too many of them, we could modify them to use one or
> > two user options.  For starters, these options would need to be set
> > manually, by the user who knows what filesystem she is working with.
> > Later, we could try to set them automatically.
> 
> I'll try this.

I did a bit of research.

Getting most things I care about to work is done by simplifying
include/filenames.h so it unconditionally takes the DOS versions of
IS_DIR_SEPARATOR() and IS_ABSOLUTE_PATH().

I also manually hacked libiberty/config.h in my build directory to
define HAVE_DOS_BASED_FILE_SYSTEM.

A quick test (yeah I know, this proves very little) shows that the case
I am after (to get DLL functionality to work) gets fixed with the
include/filenames.h change, even without the libiberty/config.h hack.

Apart from that, HAVE_DOS_BASED_FILE_SYSTEM is used in a number of
sources (even though filenames.h appears to do much already) :
./libiberty/make-relative-prefix.c
./libiberty/basename.c
./libiberty/lbasename.c
./include/filenames.h
./bfd/archive.c
./gdb/utils.c
./gdb/cli/cli-cmds.c
./gdb/completer.c
./gdb/symtab.c
./gdb/source.c

Quick analysis :

The code in bfd/archive.c could probably be cleaned up based on
IS_DIR_SEPARATOR. The code in gdb/utils.c could just always be compiled
in. Same, I think, for gdb/cli/cli-cmds.c.

One part of gdb/completer.c is file name break characters. Not sure how
to deal with those. The other #if is no problem.

The stuff in gdb/symtab.c does case independent string compare in the
DOS case.

gdb/source.c looks safe too.

My gut feeling is to just do the simplification, and add a variable that
says what to do with case insensitive file names. Don't know what to do
with the file name break characters.

But I may be oversimplifying matters.

	Danny

-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-09-01 18:03                   ` Danny Backx
@ 2009-09-01 19:34                     ` Eli Zaretskii
  2009-09-01 20:10                       ` Danny Backx
  2009-09-01 19:35                     ` Eli Zaretskii
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2009-09-01 19:34 UTC (permalink / raw)
  To: danny.backx; +Cc: tromey, gdb

> From: Danny Backx <danny.backx@scarlet.be>
> Cc: tromey@redhat.com, gdb@sourceware.org
> Date: Tue, 01 Sep 2009 20:04:55 +0200
> 
> Getting most things I care about to work is done by simplifying
> include/filenames.h so it unconditionally takes the DOS versions of
> IS_DIR_SEPARATOR() and IS_ABSOLUTE_PATH().

If Unix users will not be too unhappy about that, as Daniel says, I
don't mind.  But I still think it's a better idea to have a
user-settable option to control that at runtime.

> Apart from that, HAVE_DOS_BASED_FILE_SYSTEM is used in a number of
> sources (even though filenames.h appears to do much already) :
> ./libiberty/make-relative-prefix.c
> ./libiberty/basename.c
> ./libiberty/lbasename.c
> ./include/filenames.h
> ./bfd/archive.c
> ./gdb/utils.c
> ./gdb/cli/cli-cmds.c
> ./gdb/completer.c
> ./gdb/symtab.c
> ./gdb/source.c
> 
> Quick analysis :
> 
> The code in bfd/archive.c could probably be cleaned up based on
> IS_DIR_SEPARATOR.

Even better, just use lbasename or even basename.

> The code in gdb/utils.c could just always be compiled in.

Again, if Unix users don't mind losing support for files of the form
"d:foo".

> Same, I think, for gdb/cli/cli-cmds.c.

No, this one needs another macro: FILENAME_PREFIX_LEN, which should be
1 on Posix platforms and 3 on DOS-ish systems.  Use the DOS-ish
variant on Posix systems if "d:/foo" is not an important file name.

> One part of gdb/completer.c is file name break characters. Not sure how
> to deal with those.

That one needs to be left as-is on DOS/Windows, but not on Unix.  But
this is just for completion, so it's not a grave problem for your
use-case.

> The other #if is no problem.

Again, we need FILENAME_PREFIX_LEN here.  See above.

> The stuff in gdb/symtab.c does case independent string compare in the
> DOS case.

I see a need for FILENAME_CMPN here, similar to FILENAME_CMP that we
already have.  But using this on Unix is a bad idea, I think: you do
NOT want case-insensitive file comparison there, do you?

> gdb/source.c looks safe too.

FILENAME_PREFIX_LEN again for the first hunk, something like realpath
for the second, I think.  Do we care about "d:" on Posix systems?


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-09-01 18:03                   ` Danny Backx
  2009-09-01 19:34                     ` Eli Zaretskii
@ 2009-09-01 19:35                     ` Eli Zaretskii
  1 sibling, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2009-09-01 19:35 UTC (permalink / raw)
  To: danny.backx; +Cc: tromey, gdb

> From: Danny Backx <danny.backx@scarlet.be>
> Cc: tromey@redhat.com, gdb@sourceware.org
> Date: Tue, 01 Sep 2009 20:04:55 +0200
> 
> Apart from that, HAVE_DOS_BASED_FILE_SYSTEM is used in a number of
> sources (even though filenames.h appears to do much already) :
> ./libiberty/make-relative-prefix.c
> ./libiberty/basename.c
> ./libiberty/lbasename.c
> ./include/filenames.h
> ./bfd/archive.c
> ./gdb/utils.c
> ./gdb/cli/cli-cmds.c
> ./gdb/completer.c
> ./gdb/symtab.c
> ./gdb/source.c

One more thing: libiberty is a bit tricky, because it is used in many
projects apart of GDB, which might have their own ideas about the
appropriateness of using the DOS file-name semantics.  So I would
leave that alone in the first approximation.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-09-01 19:34                     ` Eli Zaretskii
@ 2009-09-01 20:10                       ` Danny Backx
  2009-09-02  3:25                         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Danny Backx @ 2009-09-01 20:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb

On Tue, 2009-09-01 at 22:31 +0300, Eli Zaretskii wrote:
> > From: Danny Backx <danny.backx@scarlet.be>
> > Cc: tromey@redhat.com, gdb@sourceware.org
> > Date: Tue, 01 Sep 2009 20:04:55 +0200
> > 
> > Getting most things I care about to work is done by simplifying
> > include/filenames.h so it unconditionally takes the DOS versions of
> > IS_DIR_SEPARATOR() and IS_ABSOLUTE_PATH().
> 
> If Unix users will not be too unhappy about that, as Daniel says, I
> don't mind.  But I still think it's a better idea to have a
> user-settable option to control that at runtime.

I'm not opposed to introducing user-settable options, I just phrased the
analysis based on what I saw.

> > The code in bfd/archive.c could probably be cleaned up based on
> > IS_DIR_SEPARATOR.
> Even better, just use lbasename or even basename.

Except (see also your other mail) this is in libiberty.

> > Same, I think, for gdb/cli/cli-cmds.c.
> 
> No, this one needs another macro: FILENAME_PREFIX_LEN, which should be
> 1 on Posix platforms and 3 on DOS-ish systems.  Use the DOS-ish
> variant on Posix systems if "d:/foo" is not an important file name.

Should I then look into :
- changing HAVE_DOS_BASED_FILE_SYSTEM into a variable
- doing the same thing for FILENAME_PREFIX_LEN and FILENAME_CMPN

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-09-01 20:10                       ` Danny Backx
@ 2009-09-02  3:25                         ` Eli Zaretskii
  2009-09-05  9:33                           ` Danny Backx
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2009-09-02  3:25 UTC (permalink / raw)
  To: danny.backx; +Cc: tromey, gdb

> From: Danny Backx <danny.backx@scarlet.be>
> Cc: tromey@redhat.com, gdb@sourceware.org
> Date: Tue, 01 Sep 2009 22:12:08 +0200
> 
> Should I then look into :
> - changing HAVE_DOS_BASED_FILE_SYSTEM into a variable
> - doing the same thing for FILENAME_PREFIX_LEN and FILENAME_CMPN

HAVE_DOS_BASED_FILE_SYSTEM should be controllable by a variable, and
should then automatically DTRT with FILENAME_PREFIX_LEN.  As for
FILENAME_CMPN, I don't think it's appropriate for Posix systems to
compare file names case-insensitively, so if you need that as well, I
think a separate variable is in order.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: Build question
  2009-09-02  3:25                         ` Eli Zaretskii
@ 2009-09-05  9:33                           ` Danny Backx
  0 siblings, 0 replies; 19+ messages in thread
From: Danny Backx @ 2009-09-05  9:33 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb

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

On Wed, 2009-09-02 at 06:23 +0300, Eli Zaretskii wrote:
> > From: Danny Backx <danny.backx@scarlet.be>
> > Cc: tromey@redhat.com, gdb@sourceware.org
> > Date: Tue, 01 Sep 2009 22:12:08 +0200
> > 
> > Should I then look into :
> > - changing HAVE_DOS_BASED_FILE_SYSTEM into a variable
> > - doing the same thing for FILENAME_PREFIX_LEN and FILENAME_CMPN
> 
> HAVE_DOS_BASED_FILE_SYSTEM should be controllable by a variable, and
> should then automatically DTRT with FILENAME_PREFIX_LEN.  As for
> FILENAME_CMPN, I don't think it's appropriate for Posix systems to
> compare file names case-insensitively, so if you need that as well, I
> think a separate variable is in order.

ESR said "early and often", right ? Here is a first draft of my work.
Showing this early allows you to steer me in the right direction.

I've changed include/filenames.h and the gdb source files that contained
HAVE_DOS_BASED_FILE_SYSTEM to replace that by a variable.

Some surprises :
- libiberty also includes filenames.h
- filenames.h says it's part of bfd, so basing it on a global variable,
  as I did, might have side effects

Otherwise, this code appears to work for me. I hardcoded the variable in
gdb/main.c, set this to both 0 and 1 for testing, and saw the expected
results.

Comments on all this welcome (e.g. I've left all this stuff in the
include file as inline functions, is this a bad idea?).

Also I left all the case sensitive stuff alone, and the also the
filename break characters, so HAVE_DOS_BASED_FILE_SYSTEM is still
present (gdb/symtab.c and gdb/completer.c).

Should I copy code that handles a variable like "solib-search-path" or
"annotate" to set the _have_dos_based_file_system at runtime ?

	Danny
-- 
Danny Backx ; danny.backx - at - scarlet.be ; http://danny.backx.info

[-- Attachment #2: gdb-diff-dos-1 --]
[-- Type: text/x-patch, Size: 7963 bytes --]

Index: gdb/completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.34
diff -u -r1.34 completer.c
--- gdb/completer.c	25 Mar 2009 10:50:56 -0000	1.34
+++ gdb/completer.c	5 Sep 2009 09:23:13 -0000
@@ -232,13 +232,12 @@
 	  else
 	    break;		/* Hit the end of text.  */
 	}
-#if HAVE_DOS_BASED_FILE_SYSTEM
       /* If we have a DOS-style absolute file name at the beginning of
 	 TEXT, and the colon after the drive letter is the only colon
 	 we found, pretend the colon is not there.  */
-      else if (p < text + 3 && *p == ':' && p == text + 1 + quoted)
+      else if (_have_dos_based_file_system
+            && p < text + 3 && *p == ':' && p == text + 1 + quoted)
 	;
-#endif
       else if (*p == ':' && !colon)
 	{
 	  colon = p;
Index: gdb/main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.77
diff -u -r1.77 main.c
--- gdb/main.c	27 Aug 2009 21:56:38 -0000	1.77
+++ gdb/main.c	5 Sep 2009 09:23:13 -0000
@@ -67,6 +67,9 @@
 /* GDB datadir, used to store data files.  */
 char *gdb_datadir = 0;
 
+/* Filesystem type */
+int _have_dos_based_file_system = 1;
+
 struct ui_file *gdb_stdout;
 struct ui_file *gdb_stderr;
 struct ui_file *gdb_stdlog;
Index: gdb/source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.103
diff -u -r1.103 source.c
--- gdb/source.c	23 Jul 2009 23:20:00 -0000	1.103
+++ gdb/source.c	5 Sep 2009 09:23:14 -0000
@@ -474,14 +474,20 @@
       /* name is the start of the directory.
 	 p is the separator (or null) following the end.  */
 
-      while (!(IS_DIR_SEPARATOR (*name) && p <= name + 1)	/* "/" */
-#ifdef HAVE_DOS_BASED_FILE_SYSTEM
-      /* On MS-DOS and MS-Windows, h:\ is different from h: */
-	     && !(p == name + 3 && name[1] == ':')		/* "d:/" */
-#endif
-	     && IS_DIR_SEPARATOR (p[-1]))
-	/* Sigh. "foo/" => "foo" */
-	--p;
+      if (_have_dos_based_file_system) {
+        while (!(IS_DIR_SEPARATOR (*name) && p <= name + 1)	/* "/" */
+        /* On MS-DOS and MS-Windows, h:\ is different from h: */
+	       && !(p == name + 3 && name[1] == ':')		/* "d:/" */
+	       && IS_DIR_SEPARATOR (p[-1]))
+	  /* Sigh. "foo/" => "foo" */
+	  --p;
+      } else {
+        while (!(IS_DIR_SEPARATOR (*name) && p <= name + 1)	/* "/" */
+        /* On MS-DOS and MS-Windows, h:\ is different from h: */
+	       && IS_DIR_SEPARATOR (p[-1]))
+	  /* Sigh. "foo/" => "foo" */
+	  --p;
+      }
       *p = '\0';
 
       while (p > name && p[-1] == '.')
@@ -514,10 +520,9 @@
 
       if (name[0] == '~')
 	name = tilde_expand (name);
-#ifdef HAVE_DOS_BASED_FILE_SYSTEM
-      else if (IS_ABSOLUTE_PATH (name) && p == name + 2) /* "d:" => "d:." */
+      else if (_have_dos_based_file_system
+	    && IS_ABSOLUTE_PATH (name) && p == name + 2) /* "d:" => "d:." */
 	name = concat (name, ".", (char *)NULL);
-#endif
       else if (!IS_ABSOLUTE_PATH (name) && name[0] != '$')
 	name = concat (current_directory, SLASH_STRING, name, (char *)NULL);
       else
Index: gdb/utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.219
diff -u -r1.219 utils.c
--- gdb/utils.c	18 Aug 2009 16:17:16 -0000	1.219
+++ gdb/utils.c	5 Sep 2009 09:23:14 -0000
@@ -3280,15 +3280,14 @@
   strncpy (dir_name, filename, base_name - filename);
   dir_name[base_name - filename] = '\000';
 
-#ifdef HAVE_DOS_BASED_FILE_SYSTEM
   /* We need to be careful when filename is of the form 'd:foo', which
      is equivalent of d:./foo, which is totally different from d:/foo.  */
-  if (strlen (dir_name) == 2 && isalpha (dir_name[0]) && dir_name[1] == ':')
+  if (_have_dos_based_file_system
+   && strlen (dir_name) == 2 && isalpha (dir_name[0]) && dir_name[1] == ':')
     {
       dir_name[2] = '.';
       dir_name[3] = '\000';
     }
-#endif
 
   /* Canonicalize the directory prefix, and build the resulting
      filename. If the dirname realpath already contains an ending
Index: gdb/cli/cli-cmds.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v
retrieving revision 1.92
diff -u -r1.92 cli-cmds.c
--- gdb/cli/cli-cmds.c	11 Jul 2009 14:04:23 -0000	1.92
+++ gdb/cli/cli-cmds.c	5 Sep 2009 09:23:14 -0000
@@ -357,24 +357,26 @@
   if (chdir (dir) < 0)
     perror_with_name (dir);
 
-#ifdef HAVE_DOS_BASED_FILE_SYSTEM
   /* There's too much mess with DOSish names like "d:", "d:.",
      "d:./foo" etc.  Instead of having lots of special #ifdef'ed code,
      simply get the canonicalized name of the current directory.  */
-  dir = getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
-#endif
+  if (_have_dos_based_file_system)
+    dir = getcwd (gdb_dirbuf, sizeof (gdb_dirbuf));
 
   len = strlen (dir);
   if (IS_DIR_SEPARATOR (dir[len - 1]))
     {
       /* Remove the trailing slash unless this is a root directory
          (including a drive letter on non-Unix systems).  */
-      if (!(len == 1)		/* "/" */
-#ifdef HAVE_DOS_BASED_FILE_SYSTEM
-	  && !(len == 3 && dir[1] == ':') /* "d:/" */
-#endif
-	  )
-	len--;
+      if (_have_dos_based_file_system) {
+        if (!(len == 1)		/* "/" */
+	    && !(len == 3 && dir[1] == ':') /* "d:/" */
+	    )
+	  len--;
+      } else {
+        if (!(len == 1))		/* "/" */
+	  len--;
+      }
     }
 
   dir = savestring (dir, len);
Index: include/filenames.h
===================================================================
RCS file: /cvs/src/src/include/filenames.h,v
retrieving revision 1.5
diff -u -r1.5 filenames.h
--- include/filenames.h	21 Mar 2008 23:40:18 -0000	1.5
+++ include/filenames.h	5 Sep 2009 09:23:15 -0000
@@ -5,7 +5,7 @@
    use forward- and back-slash in path names interchangeably, and
    some of them have case-insensitive file names.
 
-   Copyright 2000, 2001, 2007 Free Software Foundation, Inc.
+   Copyright 2000, 2001, 2007, 2009 Free Software Foundation, Inc.
 
 This file is part of BFD, the Binary File Descriptor library.
 
@@ -30,25 +30,54 @@
 extern "C" {
 #endif
 
-#if defined(__MSDOS__) || defined(_WIN32) || defined(__OS2__) || defined (__CYGWIN__)
-
-#ifndef HAVE_DOS_BASED_FILE_SYSTEM
-#define HAVE_DOS_BASED_FILE_SYSTEM 1
+#ifndef FALSE
+#define FALSE 0
+#endif
+#ifndef TRUE
+#define TRUE 1
 #endif
 
-#define IS_DIR_SEPARATOR(c)	((c) == '/' || (c) == '\\')
-/* Note that IS_ABSOLUTE_PATH accepts d:foo as well, although it is
-   only semi-absolute.  This is because the users of IS_ABSOLUTE_PATH
-   want to know whether to prepend the current working directory to
-   a file name, which should not be done with a name like d:foo.  */
-#define IS_ABSOLUTE_PATH(f)	(IS_DIR_SEPARATOR((f)[0]) || (((f)[0]) && ((f)[1] == ':')))
-
-#else  /* not DOSish */
+/*
+ * Defined in gdb/main.c
+ *
+ * This determines whether we have
+ *   as a separator : / or \
+ *   a prefix [a-z]: or not
+ * Replaces HAVE_DOS_BASED_FILE_SYSTEM and FILENAME_PREFIX_LEN.
+ *
+ * Case sensitive/insensitive file name comparison is *not* influenced by this.
+ */
+
+extern int _have_dos_based_file_system;
+
+static inline int _isalpha(int c)
+{ 
+  if (c <= 'Z' && c >= 'A')
+    return TRUE;
+  if (c <= 'z' && c >= 'a')
+    return TRUE;
+  return FALSE;
+}
 
-#define IS_DIR_SEPARATOR(c)	((c) == '/')
-#define IS_ABSOLUTE_PATH(f)	(IS_DIR_SEPARATOR((f)[0]))
+static inline int IS_DIR_SEPARATOR(int c)
+{
+  if (_have_dos_based_file_system) {
+    return (c == '/' || c == '\\');
+  } else {
+    return (c == '/');
+  }
+}
 
-#endif /* not DOSish */
+static inline int IS_ABSOLUTE_PATH(const char *f)
+{
+  if (IS_DIR_SEPARATOR(f[0]))
+    return TRUE;
+  if (_have_dos_based_file_system) {
+    if (_isalpha(f[0]) && f[1] == ':')
+      return TRUE;
+  }
+  return FALSE;
+}
 
 extern int filename_cmp (const char *s1, const char *s2);
 #define FILENAME_CMP(s1, s2)	filename_cmp(s1, s2)

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2009-09-05  9:33 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-21  0:24 Build question Danny Backx
2009-08-21 17:30 ` Tom Tromey
2009-08-21 17:57   ` Eli Zaretskii
2009-08-21 18:23     ` Danny Backx
2009-08-21 18:51       ` Eli Zaretskii
2009-08-21 20:15         ` Danny Backx
2009-08-21 21:57           ` Eli Zaretskii
2009-08-22  9:03           ` Tom Tromey
2009-08-22  9:55             ` Danny Backx
2009-08-22 11:07               ` Eli Zaretskii
2009-08-24 10:11                 ` Danny Backx
2009-09-01 18:03                   ` Danny Backx
2009-09-01 19:34                     ` Eli Zaretskii
2009-09-01 20:10                       ` Danny Backx
2009-09-02  3:25                         ` Eli Zaretskii
2009-09-05  9:33                           ` Danny Backx
2009-09-01 19:35                     ` Eli Zaretskii
2009-08-22 22:01             ` Daniel Jacobowitz
2009-08-23  7:28               ` Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox