Re: [RFA] Remove OS-specific defines (was: _WIN32?)

Re: [RFA] Remove OS-specific defines (was: _WIN32?)

[David Taylor]

    Date: Mon, 7 May 2001 19:10:59 +0300 (IDT)
    From: Eli Zaretskii <eliz@is.elta.co.il>

    I'm seeking approval for the following patches.  They remove all the
    DOS- and Windows-specific #ifdef's like "#ifdef _WIN32" and use the
    portable macros from filenames.h instead.  In addition, they convert
    code which used macros private to GDB, such as SLASH_P and ROOTED_P,
    to use the macros from filenames.h instead.

I was surprised that you sent this to gdb rather than gdb-patches.  I
added gdb-patches to the cc list.

    2001-05-07  Eli Zaretskii  <eliz@is.elta.co.il>

	    * source.c (mod_path, openp): Use HAVE_DOS_BASED_FILE_SYSTEM
	    instead of system-specific define's like _WIN32 and __MSDOS__.
	    Use IS_DIR_SEPARATOR and IS_ABSOLUTE_PATH instead of SLASH_P and
	    ROOTED_P.
	    (top-level): #include "filenames.h".

	    * solib.c (solib_open): Use IS_DIR_SEPARATOR and IS_ABSOLUTE_PATH
	    instead of SLASH_CHAR, ROOTED_P and SLASH_P.
	    (top-level): #include "filenames.h".

	    * defs.h (SLASH_P, SLASH_CHAR, ROOTED_P): Remove definitions.
	    (SLASH_STRING): Define only for _WIN32.

The defs.h part of this patch is approved.

	    * completer.c: Use HAVE_DOS_BASED_FILE_SYSTEM instead of
	    __MSDOS_.

	    * cli/cli-cmds.c (cd_command): Use IS_DIR_SEPARATOR and
	    IS_ABSOLUTE_PATH instead of SLASH_P and ROOTED_P.  Replace
	    system-specific ifdefs with HAVE_DOS_BASED_FILE_SYSTEM.
	    (top-level): #include "filenames.h".

Re: [Mingw-users] Re: _WIN32?

[Danny Smith]

--- Andrew Cagney <ac131313@cygnus.com> wrote: > > ACT, Inc. is also committed
to producing or assisting in a MINGW32
> > port of GDB5, and contributing the necessary patches.  However, I
> > don't have a timetable for this yet.
> > 
> > In any case, again this argues against removing the _WIN32 conditionals.
> 
> I'm not so sure.  It depends on why/how the tests are there. For instance:
> 
> 
> #if defined(_WIN32) || defined(__MSDOS__) || defined(__CYGWIN__):
> 
> Remember, GDB is moving away from OS specific tests and towards 
> autoconf/feature based tests.  Things like this should be replaced by

> something like WITH_REALLY_TRICKY_FILESYSTEM_SUPPORT test (and 
> eventually changed to a runtime switch?).
> 
> 
> main.c:
>    /* The default command loop.
>       The WIN32 Gui calls this main to set up gdb's state, and
>       has its own command loop. */
> #if !defined _WIN32 || defined __GNUC__
>    /* GUIs generally have their own command loop, mainloop, or
>       whatever.  This is a good place to gain control because many
>       error conditions will end up here via longjmp(). */
> The _WIN32 test here is probably for wingdb (and old, long dead, MS 
> Windows based GDB) and not for mingw.  There are plenty of other 
> examples of the same thing.
> 
> For reference, there are ~38 uses of _WIN32 in the sources, ~18 don't 
> appear to be accompanied by #ifdef MSDOS, CYGWIN or GO32. It is those 18 
> that I think should be deleted.
> 
> 	Andrew
> 
>
In trying to get GDB to work under mingw, I have found the #if(n)def _WIN32 
useful as tags to look for potential trouble spots, but let me ask you to
clarify something:

You are proposing that something like this in inflow.c 

#ifndef _WIN32
  kill (PIDGET (inferior_pid), SIGINT);
#endif

would be replaced with something like this:

#ifndef _N0_KILL_
  kill (PIDGET (inferior_pid), SIGINT);
#endif

with configure defining _NO_KILL for me.

Is that correct?  If so, then I have no objections to replacing the _WIN32
conditionals with something more specific.

Danny

_____________________________________________________________________________
http://store.yahoo.com.au - Yahoo! Store
- It's time you had your business online!

Re: [Mingw-users] Re: _WIN32?

[Andrew Cagney]

For:

> #ifndef _WIN32
>   kill (PIDGET (inferior_pid), SIGINT);
> #endif
> 
> would be replaced with something like this:
> 
> #ifndef _N0_KILL_
> kill (PIDGET (inferior_pid), SIGINT);
> #endif

I'm actually puzzled.  It looks very like someone took a shortcut and, 
instead of implementing new methods, just went through and commented out 
every call to kill().  cf handle_sigio().

Anyway, to answer your question:

> You are proposing that something like this in inflow.c 

Yes, for the most part I would like to strongly encouraging people doing 
WIN32, GO32 and CYGWIN ports to look back over all those #ifdef's and 
see if they are better served by an autoconf feature test. Per my 
earlier e-mail, the obvious oneis to do with how file systems are DOS 
file systems are handled and there, I think things are being replaced 
with a runtime test.

There is also at least one specific case, main.c, where I think the 
_WIN32 test is bogus.

   enjoy,
	Andrew

[RFA] Remove OS-specific defines (was: _WIN32?)

[Eli Zaretskii]

I'm seeking approval for the following patches.  They remove all the
DOS- and Windows-specific #ifdef's like "#ifdef _WIN32" and use the
portable macros from filenames.h instead.  In addition, they convert
code which used macros private to GDB, such as SLASH_P and ROOTED_P,
to use the macros from filenames.h instead.


2001-05-07  Eli Zaretskii  <eliz@is.elta.co.il>

	* source.c (mod_path, openp): Use HAVE_DOS_BASED_FILE_SYSTEM
	instead of system-specific define's like _WIN32 and __MSDOS__.
	Use IS_DIR_SEPARATOR and IS_ABSOLUTE_PATH instead of SLASH_P and
	ROOTED_P.
	(top-level): #include "filenames.h".

	* solib.c (solib_open): Use IS_DIR_SEPARATOR and IS_ABSOLUTE_PATH
	instead of SLASH_CHAR, ROOTED_P and SLASH_P.
	(top-level): #include "filenames.h".

	* defs.h (SLASH_P, SLASH_CHAR, ROOTED_P): Remove definitions.
	(SLASH_STRING): Define only for _WIN32.

	* completer.c: Use HAVE_DOS_BASED_FILE_SYSTEM instead of
	__MSDOS_.

	* cli/cli-cmds.c (cd_command): Use IS_DIR_SEPARATOR and
	IS_ABSOLUTE_PATH instead of SLASH_P and ROOTED_P.  Replace
	system-specific ifdefs with HAVE_DOS_BASED_FILE_SYSTEM.
	(top-level): #include "filenames.h".


--- gdb/cli/cli-cmds.c~0	Thu Mar 29 01:42:22 2001
+++ gdb/cli/cli-cmds.c	Mon May  7 18:08:40 2001
@@ -23,6 +23,7 @@
 #include "target.h"	 /* For baud_rate, remote_debug and remote_timeout */
 #include "gdb_wait.h"		/* For shell escape implementation */
 #include "gnu-regex.h"		/* Used by apropos_command */
+#include "filenames.h"		/* for DOSish file names */
 
 #ifdef UI_OUT
 #include "ui-out.h"
@@ -292,7 +293,7 @@ cd_command (char *dir, int from_tty)
   if (chdir (dir) < 0)
     perror_with_name (dir);
 
-#if defined(_WIN32) || defined(__MSDOS__)
+#if 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.  */
@@ -300,24 +301,24 @@ cd_command (char *dir, int from_tty)
 #endif
 
   len = strlen (dir);
-  if (SLASH_P (dir[len - 1]))
+  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)		/* "/" */
-#if defined(_WIN32) || defined(__MSDOS__)
-	  && !(!SLASH_P (*dir) && ROOTED_P (dir) && len <= 3)	/* "d:/" */
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	  && !(len == 3 && dir[1] == ':') /* "d:/" */
 #endif
 	  )
 	len--;
     }
 
   dir = savestring (dir, len);
-  if (ROOTED_P (dir))
+  if (IS_ABSOLUTE_PATH (dir))
     current_directory = dir;
   else
     {
-      if (SLASH_P (current_directory[strlen (current_directory) - 1]))
+      if (IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1]))
 	current_directory = concat (current_directory, dir, NULL);
       else
 	current_directory = concat (current_directory, SLASH_STRING, dir, NULL);
@@ -329,17 +330,18 @@ cd_command (char *dir, int from_tty)
   found_real_path = 0;
   for (p = current_directory; *p;)
     {
-      if (SLASH_P (p[0]) && p[1] == '.' && (p[2] == 0 || SLASH_P (p[2])))
+      if (IS_DIR_SEPARATOR (p[0]) && p[1] == '.'
+	  && (p[2] == 0 || IS_DIR_SEPARATOR (p[2])))
 	strcpy (p, p + 2);
-      else if (SLASH_P (p[0]) && p[1] == '.' && p[2] == '.'
-	       && (p[3] == 0 || SLASH_P (p[3])))
+      else if (IS_DIR_SEPARATOR (p[0]) && p[1] == '.' && p[2] == '.'
+	       && (p[3] == 0 || IS_DIR_SEPARATOR (p[3])))
 	{
 	  if (found_real_path)
 	    {
 	      /* Search backwards for the directory just before the "/.."
 	         and obliterate it and the "/..".  */
 	      char *q = p;
-	      while (q != current_directory && !SLASH_P (q[-1]))
+	      while (q != current_directory && !IS_DIR_SEPARATOR (q[-1]))
 		--q;
 
 	      if (q == current_directory)

--- gdb/completer.c~5	Sun May  6 13:10:30 2001
+++ gdb/completer.c	Mon May  7 17:21:02 2001
@@ -66,7 +66,7 @@ static char *gdb_completer_command_word_
    break characters any characters that are commonly used in file
    names, such as '-', '+', '~', etc.  Otherwise, readline displays
    incorrect completion candidates.  */
-#ifdef __MSDOS__
+#if HAVE_DOS_BASED_FILE_SYSTEM
 /* MS-DOS and MS-Windows use colon as part of the drive spec, and most
    programs support @foo style response files.  */
 static char *gdb_completer_file_name_break_characters = " \t\n*|\"';?><@";

--- gdb/defs.h~0	Sat Mar 24 02:00:36 2001
+++ gdb/defs.h	Mon May  7 18:08:48 2001
@@ -1330,34 +1330,14 @@ extern int use_windows;
 #define DIRNAME_SEPARATOR ':'
 #endif
 
-#ifndef SLASH_P
-#if defined(__GO32__)||defined(_WIN32)
-#define SLASH_P(X) ((X)=='\\')
-#else
-#define SLASH_P(X) ((X)=='/')
-#endif
-#endif
-
-#ifndef SLASH_CHAR
-#if defined(__GO32__)||defined(_WIN32)
-#define SLASH_CHAR '\\'
-#else
-#define SLASH_CHAR '/'
-#endif
-#endif
-
 #ifndef SLASH_STRING
-#if defined(__GO32__)||defined(_WIN32)
+#ifdef _WIN32
 #define SLASH_STRING "\\"
 #else
 #define SLASH_STRING "/"
 #endif
 #endif
 
-#ifndef ROOTED_P
-#define ROOTED_P(X) (SLASH_P((X)[0]))
-#endif
-
 /* On some systems, PIDGET is defined to extract the inferior pid from
    an internal pid that has the thread id and pid in seperate bit
    fields.  If not defined, then just use the entire internal pid as

--- gdb/solib.c~0	Tue Mar  6 10:41:56 2001
+++ gdb/solib.c	Mon May  7 18:11:24 2001
@@ -39,6 +39,7 @@
 #include "language.h"
 #include "gdbcmd.h"
 #include "completer.h"
+#include "filenames.h"		/* for DOSish file names */
 
 #include "solist.h"
 
@@ -101,10 +102,14 @@ solib_open (char *in_pathname, char **fo
 {
   int found_file = -1;
   char *temp_pathname = NULL;
+  char *p = in_pathname;
 
-  if (strchr (in_pathname, SLASH_CHAR))
+  while (*p && !IS_DIR_SEPARATOR (*p))
+    p++;
+
+  if (*p)
     {
-      if (! ROOTED_P (in_pathname) || solib_absolute_prefix == NULL)
+      if (! IS_ABSOLUTE_PATH (in_pathname) || solib_absolute_prefix == NULL)
         temp_pathname = in_pathname;
       else
 	{
@@ -112,7 +117,7 @@ solib_open (char *in_pathname, char **fo
 
 	  /* Remove trailing slashes from absolute prefix.  */
 	  while (prefix_len > 0
-		 && SLASH_P (solib_absolute_prefix[prefix_len - 1]))
+		 && IS_DIR_SEPARATOR (solib_absolute_prefix[prefix_len - 1]))
 	    prefix_len--;
 
 	  /* Cat the prefixed pathname together.  */

--- gdb/source.c~0	Tue Mar 27 22:57:04 2001
+++ gdb/source.c	Mon May  7 18:26:20 2001
@@ -40,6 +40,7 @@
 #include "annotate.h"
 #include "gdbtypes.h"
 #include "linespec.h"
+#include "filenames.h"		/* for DOSish file names */
 #ifdef UI_OUT
 #include "ui-out.h"
 #endif
@@ -330,12 +331,12 @@ mod_path (char *dirname, char **which_pa
 	  }
       }
 
-      if (!(SLASH_P (*name) && p <= name + 1)	/* "/" */
-#if defined(_WIN32) || defined(__MSDOS__)
+      if (!(IS_DIR_SEPARATOR (*name) && p <= name + 1)	 /* "/" */
+#if HAVE_DOS_BASED_FILE_SYSTEM
       /* On MS-DOS and MS-Windows, h:\ is different from h: */
-	  && !(!SLASH_P (*name) && ROOTED_P (name) && p <= name + 3)	/* d:/ */
+	  && !(p == name + 3 && name[1] == ':') 	 /* "d:/" */
 #endif
-	  && SLASH_P (p[-1]))
+	  && IS_DIR_SEPARATOR (p[-1]))
 	/* Sigh. "foo/" => "foo" */
 	--p;
       *p = '\0';
@@ -348,7 +349,7 @@ mod_path (char *dirname, char **which_pa
 	      name = current_directory;
 	      goto append;
 	    }
-	  else if (p > name + 1 && SLASH_P (p[-2]))
+	  else if (p > name + 1 && IS_DIR_SEPARATOR (p[-2]))
 	    {
 	      if (p - name == 2)
 		{
@@ -370,11 +371,11 @@ mod_path (char *dirname, char **which_pa
 
       if (name[0] == '~')
 	name = tilde_expand (name);
-#if defined(_WIN32) || defined(__MSDOS__)
-      else if (ROOTED_P (name) && p == name + 2)	/* "d:" => "d:." */
+#if HAVE_DOS_BASED_FILE_SYSTEM
+      else if (IS_ABSOLUTE_PATH (name) && p == name + 2) /* "d:" => "d:." */
 	name = concat (name, ".", NULL);
 #endif
-      else if (!ROOTED_P (name) && name[0] != '$')
+      else if (!IS_ABSOLUTE_PATH (name) && name[0] != '$')
 	name = concat (current_directory, SLASH_STRING, name, NULL);
       else
 	name = savestring (name, p - name);
@@ -530,7 +531,7 @@ openp (char *path, int try_cwd_first, ch
   mode |= O_BINARY;
 #endif
 
-  if (try_cwd_first || ROOTED_P (string))
+  if (try_cwd_first || IS_ABSOLUTE_PATH (string))
     {
       int i;
       filename = string;
@@ -538,12 +539,12 @@ openp (char *path, int try_cwd_first, ch
       if (fd >= 0)
 	goto done;
       for (i = 0; string[i]; i++)
-	if (SLASH_P (string[i]))
+	if (IS_DIR_SEPARATOR (string[i]))
 	  goto done;
     }
 
   /* ./foo => foo */
-  while (string[0] == '.' && SLASH_P (string[1]))
+  while (string[0] == '.' && IS_DIR_SEPARATOR (string[1]))
     string += 2;
 
   alloclen = strlen (path) + strlen (string) + 2;
@@ -581,7 +582,7 @@ openp (char *path, int try_cwd_first, ch
 	}
 
       /* Remove trailing slashes */
-      while (len > 0 && SLASH_P (filename[len - 1]))
+      while (len > 0 && IS_DIR_SEPARATOR (filename[len - 1]))
 	filename[--len] = 0;
 
       strcat (filename + len, SLASH_STRING);
@@ -597,14 +598,14 @@ done:
     {
       if (fd < 0)
 	*filename_opened = (char *) 0;
-      else if (ROOTED_P (filename))
+      else if (IS_ABSOLUTE_PATH (filename))
 	*filename_opened = savestring (filename, strlen (filename));
       else
 	{
 	  /* Beware the // my son, the Emacs barfs, the botch that catch... */
 
 	  *filename_opened = concat (current_directory,
-		 SLASH_P (current_directory[strlen (current_directory) - 1])
+           IS_DIR_SEPARATOR (current_directory[strlen (current_directory) - 1])
 				     ? "" : SLASH_STRING,
 				     filename, NULL);
 	}

Re: [RFA] Remove OS-specific defines (was: _WIN32?)

[Kevin Buettner]

On May 7,  7:10pm, Eli Zaretskii wrote:

> 	* solib.c (solib_open): Use IS_DIR_SEPARATOR and IS_ABSOLUTE_PATH
> 	instead of SLASH_CHAR, ROOTED_P and SLASH_P.
> 	(top-level): #include "filenames.h".

Your changes to solib.c are approved.  (FWIW, I glanced over your
changes to the other files and they look good too.)

> 	* defs.h (SLASH_P, SLASH_CHAR, ROOTED_P): Remove definitions.
> 	(SLASH_STRING): Define only for _WIN32.

Once your changes to the other .c files are approved, I think this
one can be regarded as an obvious change.

Don't forget to also remove the definitions of SLASH_P, SLASH_CHAR, and
ROOTED_P in config/i386/xm-cygwin.h and config/i386/xm-go32.h.

Kevin

Re: [RFA] Remove OS-specific defines (was: _WIN32?)

[Eli Zaretskii]

> Date: Mon, 7 May 2001 10:38:55 -0700
> From: Kevin Buettner <kevinb@cygnus.com>
> 
> On May 7,  7:10pm, Eli Zaretskii wrote:
> 
> > 	* solib.c (solib_open): Use IS_DIR_SEPARATOR and IS_ABSOLUTE_PATH
> > 	instead of SLASH_CHAR, ROOTED_P and SLASH_P.
> > 	(top-level): #include "filenames.h".
> 
> Your changes to solib.c are approved.  (FWIW, I glanced over your
> changes to the other files and they look good too.)

Thanks for reviewing them so soon.

> Don't forget to also remove the definitions of SLASH_P, SLASH_CHAR, and
> ROOTED_P in config/i386/xm-cygwin.h and config/i386/xm-go32.h.

Yes, it's in the pipeline ;-)  I just wanted to hear from Chris, at
the very least, before futzing with his files.

Btw, is it perhaps a good idea to add to gdbint.texinfo some
guidelines about writing portable code which handles file names,
including the list of these macros?  Andrew?

Re: [RFA] Remove OS-specific defines (was: _WIN32?)

[Andrew Cagney]

>> Don't forget to also remove the definitions of SLASH_P, SLASH_CHAR, and
>> ROOTED_P in config/i386/xm-cygwin.h and config/i386/xm-go32.h.

I think these changes are straightforward (er obvious) and definitly a 
good thing.


> Yes, it's in the pipeline ;-)  I just wanted to hear from Chris, at
> the very least, before futzing with his files.
> 
> Btw, is it perhaps a good idea to add to gdbint.texinfo some
> guidelines about writing portable code which handles file names,
> including the list of these macros?  Andrew?

Yes, and yes.

	Andrew