Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: gdb-patches@sourceware.org
Cc: Joel Brobecker <brobecker@adacore.com>
Subject: [RFA/commit/Windows] run program with space in path to exe.
Date: Sat, 20 Oct 2012 00:29:00 -0000	[thread overview]
Message-ID: <1350692925-14181-1-git-send-email-brobecker@adacore.com> (raw)

The following works...

    % gdb c:\path to exe\foo.exe
    (gdb) start

... unless a file or directory called "c:\path" or "c:\path to" exists.
This is what happens in the latter case:

    (gdb) start
    [...]
    Error creating process C:\path to exe\foo.exe (error 193).

This is because we are calling CreateProcess (et al) without specifying
the lpApplicationName, so Windows determines the name of the executable
using the second argument, which is the entire command line.  This
command line is a space-separated list of tokens, so the space in
the path to the executable which potentially creates an ambiguity.
The ambiguity is automatically resolved unless we're in the situation
above.

The solution, as suggested by the MSDN documentation for CreateProcess
is to quote the executable name.

gdb/ChangeLog:

        * windows-nat.c (windows_create_inferior) [!__CYGWIN__]:
        New local variable args_len.
        Quote the name of the executable when computing the command line.

Tested on x86-windows. Fixes the problem, with no regression.

A few remarks on the patch:

  1. As shown in the ChangeLog entry, this only deal with the MinGW
     case.  I think we have the same issue with cygwin, except it is
     a little more complicated, because there are two cases: startup
     with shell, vs startup without shell, multiplied by wide characters
     vs non-wide characters.

     It'd be nice if someone who uses the cygwin version were to fix
     the problem there as well. I don't mind writing a patch, but it
     will be just barely tested, since I do not have a testing
     environment for cygwin. I'm also waiting to see if this patch
     is accepted.

  2. It seemed much simpler to use xsnprintf rather than strcat to
     compute the command line, especially now that it has a few more
     elements in it.  So I used that instead.

  3. Theoretically, I have a feeling that we're breaking the case
     where the executable name contains a double quote in it.
     But, I think that's not something anyone would do, and I don't
     think I'd want to complexify the current code just to handle
     this theoretical case, particularly since it didn't work anyway!

         $ gdb -q 'foo"bar.exe'
         foo"bar.exe: No such file or directory.

     So, I don't think anyone is going to complain...

Any objections this patch?

Thanks,
-- 
Joel

---
 gdb/windows-nat.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 905d4bf..5eb8f67 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2036,6 +2036,7 @@ windows_create_inferior (struct target_ops *ops, char *exec_file,
   char shell[__PMAX]; /* Path to shell */
   char *toexec;
   char *args;
+  size_t args_len;
   HANDLE tty;
   char *w32env;
   char *temp;
@@ -2188,10 +2189,13 @@ windows_create_inferior (struct target_ops *ops, char *exec_file,
     }
 #else
   toexec = exec_file;
-  args = alloca (strlen (toexec) + strlen (allargs) + 2);
-  strcpy (args, toexec);
-  strcat (args, " ");
-  strcat (args, allargs);
+  /* Build the command line, a space-separated list of tokens where
+     the first token is the name of the module to be executed.
+     To avoid ambiguities introduced by spaces in the module name,
+     we quote it.  */
+  args_len = strlen (toexec) + 2 /* quotes */ + strlen (allargs) + 2;
+  args = alloca (args_len);
+  xsnprintf (args, args_len, "\"%s\" %s", toexec, allargs);
 
   flags |= DEBUG_ONLY_THIS_PROCESS;
 
-- 
1.7.9.5


             reply	other threads:[~2012-10-20  0:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-20  0:29 Joel Brobecker [this message]
2012-10-20  8:03 ` Eli Zaretskii
2012-10-20 16:30   ` Joel Brobecker
2012-10-20 17:20     ` Eli Zaretskii
2012-10-24 13:42       ` Checked in: " Joel Brobecker
2012-10-24 14:34         ` Pedro Alves
2012-10-24 14:47           ` Joel Brobecker
2012-10-24 14:57             ` Pedro Alves
2012-10-24 15:01               ` Joel Brobecker
2012-10-24 15:08                 ` Pedro Alves
2012-10-25  2:48                 ` Christopher Faylor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1350692925-14181-1-git-send-email-brobecker@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox