[PATCH] Allow spaces in pathnames to directory command

[PATCH] Allow spaces in pathnames to directory command

[Andrew STUBBS]

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

Hi,

The directory command and the -d switch do not support pathnames that 
contain spaces.

This is due to the fact that the directory command permits multiple 
paths to be added at once.

The attached patch should solve the problem by using buildargv() to 
parse the options, thus allowing quoting in the usual way.

It creates a new function, directory_switch(), for use in conjunction 
with the -d switch. This adds the path without attempting to split it 
into multiple paths - white space and quoting will have been parsed by 
the shell already.

Additionally, I have altered the existing 'foo/' => 'foo' transformation 
to remove an arbitrary number of trailing slashes.

OK?

Andrew Stubbs

[-- Attachment #2: directory_command.patch --]
[-- Type: text/plain, Size: 4762 bytes --]

2005-12-06  Andrew Stubbs  <andrew.stubbs@st.com>

	* defs.h (directory_switch): Add prototype.
	* main.c (captured_main): Use directory_switch() instead of
	directory_command() to add directories from the -d switch.
	* source.c (directory_switch): New function.
	(add_path): Use buildargv() to parse spaces in filenames properly.
	Strip multiple trailing '/' rather than just one.

Index: src/gdb/defs.h
===================================================================
--- src.orig/gdb/defs.h	2005-11-09 15:35:33.000000000 +0000
+++ src/gdb/defs.h	2005-12-06 14:18:29.000000000 +0000
@@ -611,6 +611,8 @@ extern void add_path (char *, char **, i
 
 extern void directory_command (char *, int);
 
+extern void directory_switch (char *, int);
+
 extern char *source_path;
 
 extern void init_source_path (void);
Index: src/gdb/main.c
===================================================================
--- src.orig/gdb/main.c	2005-11-29 16:35:45.000000000 +0000
+++ src/gdb/main.c	2005-12-06 14:13:53.000000000 +0000
@@ -667,7 +667,7 @@ extern int gdbtk_test (char *);
     }
 
   for (i = 0; i < ndir; i++)
-    catch_command_errors (directory_command, dirarg[i], 0, RETURN_MASK_ALL);
+    catch_command_errors (directory_switch, dirarg[i], 0, RETURN_MASK_ALL);
   xfree (dirarg);
 
   if (execarg != NULL
Index: src/gdb/source.c
===================================================================
--- src.orig/gdb/source.c	2005-08-29 13:57:49.000000000 +0100
+++ src/gdb/source.c	2005-12-06 14:55:10.000000000 +0000
@@ -378,6 +378,15 @@ directory_command (char *dirname, int fr
   forget_cached_source_info ();
 }
 
+/* Add a path given with the -d command line switch.
+   This will not be quoted so we must not treat spaces as separators.  */
+
+void
+directory_switch (char *dirname, int from_tty)
+{
+  add_path (dirname, &source_path, 0);
+}
+
 /* Add zero or more directories to the front of an arbitrary path.  */
 
 void
@@ -397,56 +406,72 @@ add_path (char *dirname, char **which_pa
 {
   char *old = *which_path;
   int prefix = 0;
+  char **argv = NULL;
+  char *arg;
+  int argv_index = 0;
 
   if (dirname == 0)
     return;
 
-  dirname = xstrdup (dirname);
-  make_cleanup (xfree, dirname);
+  if (parse_separators)
+    {
+      /* This will properly parse the space and tab separators
+	 and any quotes that may exist. DIRNAME_SEPARATOR will
+	 be dealt with later.  */
+      argv = buildargv (dirname);
+      make_cleanup_freeargv (argv);
+
+      if (argv == NULL)
+	nomem (0);
+
+      arg = argv[0];
+    }
+  else
+    {
+      arg = xstrdup (dirname);
+      make_cleanup (xfree, arg);
+    }
 
   do
     {
-      char *name = dirname;
+      char *name = arg;
       char *p;
       struct stat st;
 
       {
 	char *separator = NULL;
-	char *space = NULL;
-	char *tab = NULL;
 
+	/* Spaces and tabs will have been removed by buildargv().
+	   The directories will there be split into a list but
+	   each entry may still contain DIRNAME_SEPARATOR.  */
 	if (parse_separators)
-	  {
-	    separator = strchr (name, DIRNAME_SEPARATOR);
-	    space = strchr (name, ' ');
-	    tab = strchr (name, '\t');
-	  }
+	  separator = strchr (name, DIRNAME_SEPARATOR);
 
-	if (separator == 0 && space == 0 && tab == 0)
-	  p = dirname = name + strlen (name);
+	if (separator == 0)
+	  p = arg = name + strlen (name);
 	else
 	  {
-	    p = 0;
-	    if (separator != 0 && (p == 0 || separator < p))
-	      p = separator;
-	    if (space != 0 && (p == 0 || space < p))
-	      p = space;
-	    if (tab != 0 && (p == 0 || tab < p))
-	      p = tab;
-	    dirname = p + 1;
-	    while (*dirname == DIRNAME_SEPARATOR
-		   || *dirname == ' '
-		   || *dirname == '\t')
-	      ++dirname;
+	    p = separator;
+	    arg = p + 1;
+	    while (*arg == DIRNAME_SEPARATOR)
+	      ++arg;
 	  }
+
+	/* If there are no more directories in this argument then start
+	   on the next argument next time round the loop (if any).  */
+	if (*arg == '\0')
+	  arg = parse_separators ? argv[++argv_index] : NULL;
       }
 
-      if (!(IS_DIR_SEPARATOR (*name) && p <= name + 1)	 /* "/" */
+      /* 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:/" */
+	     && !(p == name + 3 && name[1] == ':')		/* "d:/" */
 #endif
-	  && IS_DIR_SEPARATOR (p[-1]))
+	     && IS_DIR_SEPARATOR (p[-1]))
 	/* Sigh. "foo/" => "foo" */
 	--p;
       *p = '\0';
@@ -577,7 +602,7 @@ add_path (char *dirname, char **which_pa
       }
     skip_dup:;
     }
-  while (*dirname != '\0');
+  while (arg != NULL);
 }
 
 

Re: [PATCH] Allow spaces in pathnames to directory command

[Andrew STUBBS]

Andrew Stubbs wrote:
> The directory command and the -d switch do not support pathnames that 
> contain spaces.

It looks like my patch ought to fix PR gdb/1832, PR gdb/956, and perhaps 
PR gdb/535.

Re: [PATCH] Allow spaces in pathnames to directory command

[Daniel Jacobowitz]

On Tue, Dec 06, 2005 at 03:17:07PM +0000, Andrew STUBBS wrote:
> Andrew Stubbs wrote:
> >The directory command and the -d switch do not support pathnames that 
> >contain spaces.
> 
> It looks like my patch ought to fix PR gdb/1832, PR gdb/956, and perhaps 
> PR gdb/535.

Does Andrew's comment about filename_completer in gdb/535 still apply?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Allow spaces in pathnames to directory command

[Andrew STUBBS]

Daniel Jacobowitz wrote:
> On Tue, Dec 06, 2005 at 03:17:07PM +0000, Andrew STUBBS wrote:
>> Andrew Stubbs wrote:
>>> The directory command and the -d switch do not support pathnames that 
>>> contain spaces.
>> It looks like my patch ought to fix PR gdb/1832, PR gdb/956, and perhaps 
>> PR gdb/535.
> 
> Does Andrew's comment about filename_completer in gdb/535 still apply?

Yeah, I suppose it does. Certainly the auto-completer completes names 
with spaces in (provided that the spaces are not in the part already 
typed), but does not add any form of quoting.

It doesn't quote quotes or spaces. Nor does it complete filenames that 
have quoted elements already in them.

This doesn't change the correctness of the patches in question though. 
The bug report gdb/535 complains about the behaviour of generic_load(), 
not the auto-completer - that's a separate issue raised by Andrew Cagney.

I do agree that the completer ought to be fixed at some point.

Andrew

Re: [PATCH] Allow spaces in pathnames to directory command

[Daniel Jacobowitz]

On Mon, Feb 20, 2006 at 06:06:51PM +0000, Andrew STUBBS wrote:
> Daniel Jacobowitz wrote:
> >On Tue, Dec 06, 2005 at 03:17:07PM +0000, Andrew STUBBS wrote:
> >>Andrew Stubbs wrote:
> >>>The directory command and the -d switch do not support pathnames that 
> >>>contain spaces.
> >>It looks like my patch ought to fix PR gdb/1832, PR gdb/956, and perhaps 
> >>PR gdb/535.
> >
> >Does Andrew's comment about filename_completer in gdb/535 still apply?
> 
> Yeah, I suppose it does. Certainly the auto-completer completes names 
> with spaces in (provided that the spaces are not in the part already 
> typed), but does not add any form of quoting.
> 
> It doesn't quote quotes or spaces. Nor does it complete filenames that 
> have quoted elements already in them.
> 
> This doesn't change the correctness of the patches in question though. 
> The bug report gdb/535 complains about the behaviour of generic_load(), 
> not the auto-completer - that's a separate issue raised by Andrew Cagney.
> 
> I do agree that the completer ought to be fixed at some point.

Except that previously, the filename completer produced things that
were valid for this command, and now it doesn't.  Sounds pretty
inconvenient to me.

I bumped up against this separately this morning.  I may see about
unifying the completion to handle quoting.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Allow spaces in pathnames to directory command

[Daniel Jacobowitz]

On Mon, Feb 20, 2006 at 02:19:37PM -0500, Daniel Jacobowitz wrote:
> On Mon, Feb 20, 2006 at 06:06:51PM +0000, Andrew STUBBS wrote:
> > Daniel Jacobowitz wrote:
> > >On Tue, Dec 06, 2005 at 03:17:07PM +0000, Andrew STUBBS wrote:
> > >>Andrew Stubbs wrote:
> > >>>The directory command and the -d switch do not support pathnames that 
> > >>>contain spaces.
> > >>It looks like my patch ought to fix PR gdb/1832, PR gdb/956, and perhaps 
> > >>PR gdb/535.
> > >
> > >Does Andrew's comment about filename_completer in gdb/535 still apply?
> > 
> > Yeah, I suppose it does. Certainly the auto-completer completes names 
> > with spaces in (provided that the spaces are not in the part already 
> > typed), but does not add any form of quoting.
> > 
> > It doesn't quote quotes or spaces. Nor does it complete filenames that 
> > have quoted elements already in them.
> > 
> > This doesn't change the correctness of the patches in question though. 
> > The bug report gdb/535 complains about the behaviour of generic_load(), 
> > not the auto-completer - that's a separate issue raised by Andrew Cagney.
> > 
> > I do agree that the completer ought to be fixed at some point.
> 
> Except that previously, the filename completer produced things that
> were valid for this command, and now it doesn't.  Sounds pretty
> inconvenient to me.
> 
> I bumped up against this separately this morning.  I may see about
> unifying the completion to handle quoting.

Meanwhile, I think that this is a sensible change, so go ahead and
commit it.

The story for quoting arguments passed to MI is very, very confused
right now.  I've just discovered that Eclipse doesn't quote at all,
and doesn't escape backslashes; I'm going to have to dig more, but
I'm going to try to give us a unified picture on this today.

I think that the CLI commands should expect buildargv-style quoting,
and the MI commands should take a more C-style quoting (since that's
how they are documented), but right now I fear they take both.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH] Allow spaces in pathnames to directory command

[Andrew STUBBS]

Daniel Jacobowitz wrote:
> Meanwhile, I think that this is a sensible change, so go ahead and
> commit it.

Thanks, done.

Andrew