View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003094 | GNUnet | util library | public | 2013-11-05 05:30 | 2013-12-24 20:54 |
| Reporter | bratao | Assigned To | Christian Grothoff | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | W32 | OS | Windows | OS Version | 8.1 |
| Product Version | Git master | ||||
| Target Version | 0.10.0 | Fixed in Version | 0.10.0 | ||
| Summary | 0003094: do_start_process breaks at white space | ||||
| Description | do_start_process proposely breaks whitespace to use as arguments. It is a problem in W32, as eventually one would like to install GNUnet in Program Files. Just gnunet-arm uses it. And I dont see when splitting whitespaces are useful with it. I just removed it and everything works™ now. | ||||
| Tags | No tags attached. | ||||
| Attached Files | do_start_process.c.patch (1,498 bytes)
Index: src/arm/do_start_process.c
===================================================================
--- src/arm/do_start_process.c (revision 30501)
+++ src/arm/do_start_process.c (working copy)
@@ -23,11 +23,7 @@
/**
* Actually start a process. All of the arguments given to this
* function are strings that are used for the "argv" array. However,
- * if those strings contain spaces, the given argument is split into
- * multiple argv entries without spaces. Similarly, if an argument is
- * the empty string, it is skipped. This function has the inherent
- * limitation that it does NOT allow passing command line arguments
- * with spaces to the new process.
+ * if an argument is the empty string, it is skipped.
*
* @param pipe_control should a pipe be used to send signals to the child?
* @param std_inheritance a set of GNUNET_OS_INHERIT_STD_* flags
@@ -62,14 +58,6 @@
rpos = arg;
while ('\0' != *rpos)
{
- if (' ' == *rpos)
- {
- if (last != NULL)
- argv_size++;
- last = NULL;
- while (' ' == *rpos)
- rpos++;
- }
if ((last == NULL) && (*rpos != '\0'))
last = rpos;
if (*rpos != '\0')
@@ -96,16 +84,6 @@
pos = cp;
while ('\0' != *pos)
{
- if (' ' == *pos)
- {
- *pos = '\0';
- if (last != NULL)
- argv[argv_size++] = GNUNET_strdup (last);
- last = NULL;
- pos++;
- while (' ' == *pos)
- pos++;
- }
if ((last == NULL) && (*pos != '\0'))
last = pos;
if (*pos != '\0')
do_start_process.V2.patch (5,882 bytes)
Index: src/arm/arm_api.c
===================================================================
--- src/arm/arm_api.c (revision 30501)
+++ src/arm/arm_api.c (working copy)
@@ -756,9 +756,12 @@
unsigned char test_is_active;
char *cbinary;
char *binary;
+ char *quotedbinary;
char *config;
char *loprefix;
char *lopostfix;
+ int binaryLen;
+
test_is_active = cm->h->service_test_is_active;
if ((GNUNET_YES == test_is_active) &&
@@ -814,6 +817,16 @@
cm->h->cfg, "arm", "CONFIG", &config))
config = NULL;
binary = GNUNET_OS_get_libexec_binary_path (cbinary);
+ //Surround in quotes
+ binaryLen = strlen(binary);
+ quotedbinary = GNUNET_malloc ((binaryLen + 3) * sizeof (char *));
+ quotedbinary[0] = '"';
+ strcpy("edbinary[1],binary);
+ quotedbinary[binaryLen + 1] = '"';
+ quotedbinary[binaryLen + 2] = '\0';
+
+
+
GNUNET_free (cbinary);
if ((GNUNET_YES == GNUNET_CONFIGURATION_have_value (
cm->h->cfg, "TESTING", "WEAKRANDOM")) &&
@@ -826,12 +839,12 @@
/* we're clearly running a test, don't daemonize */
if (NULL == config)
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary,
+ NULL, loprefix, quotedbinary,
/* no daemonization! */
lopostfix, NULL);
else
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary, "-c", config,
+ NULL, loprefix, quotedbinary, "-c", config,
/* no daemonization! */
lopostfix, NULL);
}
@@ -839,14 +852,15 @@
{
if (NULL == config)
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary,
+ NULL, loprefix, quotedbinary,
"-d", lopostfix, NULL);
else
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary, "-c", config,
+ NULL, loprefix, quotedbinary, "-c", config,
"-d", lopostfix, NULL);
}
GNUNET_free (binary);
+ GNUNET_free (quotedbinary);
GNUNET_free_non_null (config);
GNUNET_free (loprefix);
GNUNET_free (lopostfix);
Index: src/arm/do_start_process.c
===================================================================
--- src/arm/do_start_process.c (revision 30501)
+++ src/arm/do_start_process.c (working copy)
@@ -50,6 +50,9 @@
const char *last;
struct GNUNET_OS_Process *proc;
char *binary_path;
+ int quote_on;
+ int i;
+ int len;
argv_size = 1;
va_start (ap, first_arg);
@@ -62,8 +65,18 @@
rpos = arg;
while ('\0' != *rpos)
{
- if (' ' == *rpos)
+
+ if ('"' == *rpos)
{
+ if(quote_on == 1)
+ quote_on =0;
+ else
+ quote_on =1;
+
+ }
+
+ if ((' ' == *rpos)&&(quote_on == 0))
+ {
if (last != NULL)
argv_size++;
last = NULL;
@@ -83,6 +96,7 @@
/* *INDENT-ON* */
va_end (ap);
+ quote_on =0;
argv = GNUNET_malloc (argv_size * sizeof (char *));
argv_size = 0;
va_start (ap, first_arg);
@@ -96,8 +110,18 @@
pos = cp;
while ('\0' != *pos)
{
- if (' ' == *pos)
+
+ if ('"' == *pos)
{
+ if(quote_on == 1)
+ quote_on =0;
+ else
+ quote_on =1;
+
+ }
+
+ if ((' ' == *pos)&&(quote_on == 0))
+ {
*pos = '\0';
if (last != NULL)
argv[argv_size++] = GNUNET_strdup (last);
@@ -121,7 +145,19 @@
/* *INDENT-ON* */
va_end (ap);
argv[argv_size] = NULL;
- binary_path = argv[0];
+
+
+ for(i = 0; i < argv_size; i++){
+ len = strlen(argv[i]);
+ if((argv[i][0] == '"')&& (argv[i][len-1] == '"'))
+ {
+ argv[i][len-1] = '\0';
+ argv[i]++;
+
+ }
+ }
+ binary_path = argv[0];
+
proc = GNUNET_OS_start_process_v (pipe_control, std_inheritance, lsocks,
binary_path, argv);
while (argv_size > 0)
Index: src/arm/gnunet-service-arm.c
===================================================================
--- src/arm/gnunet-service-arm.c (revision 30501)
+++ src/arm/gnunet-service-arm.c (working copy)
@@ -414,6 +414,8 @@
SOCKTYPE *lsocks;
unsigned int ls;
char *binary;
+ char *quotedbinary;
+ int binaryLen;
/* calculate listen socket list */
lsocks = NULL;
@@ -486,6 +488,16 @@
"Starting service `%s' using binary `%s' and configuration `%s'\n",
sl->name, sl->binary, sl->config);
binary = GNUNET_OS_get_libexec_binary_path (sl->binary);
+
+ //Surround in quotes
+ binaryLen = strlen(binary);
+ quotedbinary = GNUNET_malloc ((binaryLen + 3) * sizeof (char *));
+ quotedbinary[0] = '"';
+ strcpy("edbinary[1],binary);
+ quotedbinary[binaryLen + 1] = '"';
+ quotedbinary[binaryLen + 2] = '\0';
+
+
GNUNET_assert (NULL == sl->proc);
if (GNUNET_YES == use_debug)
{
@@ -492,12 +504,12 @@
if (NULL == sl->config)
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary, "-L",
+ lsocks, loprefix, quotedbinary, "-L",
"DEBUG", options, NULL);
else
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary, "-c", sl->config, "-L",
+ lsocks, loprefix, quotedbinary, "-c", sl->config, "-L",
"DEBUG", options, NULL);
}
else
@@ -505,15 +517,16 @@
if (NULL == sl->config)
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary,
+ lsocks, loprefix, quotedbinary,
options, NULL);
else
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary, "-c", sl->config,
+ lsocks, loprefix, quotedbinary, "-c", sl->config,
options, NULL);
}
GNUNET_free (binary);
+ GNUNET_free (quotedbinary);
if (sl->proc == NULL)
{
GNUNET_log (GNUNET_ERROR_TYPE_ERROR, _("Failed to start service `%s'\n"),
do_start_process.V3.patch (5,917 bytes)
Index: src/arm/arm_api.c
===================================================================
--- src/arm/arm_api.c (revision 30501)
+++ src/arm/arm_api.c (working copy)
@@ -756,9 +756,12 @@
unsigned char test_is_active;
char *cbinary;
char *binary;
+ char *quotedbinary;
char *config;
char *loprefix;
char *lopostfix;
+ int binaryLen;
+
test_is_active = cm->h->service_test_is_active;
if ((GNUNET_YES == test_is_active) &&
@@ -814,6 +817,16 @@
cm->h->cfg, "arm", "CONFIG", &config))
config = NULL;
binary = GNUNET_OS_get_libexec_binary_path (cbinary);
+ //Surround in quotes
+ binaryLen = strlen(binary);
+ quotedbinary = GNUNET_malloc ((binaryLen + 3) * sizeof (char *));
+ quotedbinary[0] = '"';
+ strcpy("edbinary[1],binary);
+ quotedbinary[binaryLen + 1] = '"';
+ quotedbinary[binaryLen + 2] = '\0';
+
+
+
GNUNET_free (cbinary);
if ((GNUNET_YES == GNUNET_CONFIGURATION_have_value (
cm->h->cfg, "TESTING", "WEAKRANDOM")) &&
@@ -826,12 +839,12 @@
/* we're clearly running a test, don't daemonize */
if (NULL == config)
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary,
+ NULL, loprefix, quotedbinary,
/* no daemonization! */
lopostfix, NULL);
else
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary, "-c", config,
+ NULL, loprefix, quotedbinary, "-c", config,
/* no daemonization! */
lopostfix, NULL);
}
@@ -839,14 +852,15 @@
{
if (NULL == config)
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary,
+ NULL, loprefix, quotedbinary,
"-d", lopostfix, NULL);
else
proc = do_start_process (GNUNET_NO, cm->std_inheritance,
- NULL, loprefix, binary, "-c", config,
+ NULL, loprefix, quotedbinary, "-c", config,
"-d", lopostfix, NULL);
}
GNUNET_free (binary);
+ GNUNET_free (quotedbinary);
GNUNET_free_non_null (config);
GNUNET_free (loprefix);
GNUNET_free (lopostfix);
Index: src/arm/do_start_process.c
===================================================================
--- src/arm/do_start_process.c (revision 30501)
+++ src/arm/do_start_process.c (working copy)
@@ -50,6 +50,9 @@
const char *last;
struct GNUNET_OS_Process *proc;
char *binary_path;
+ int quote_on;
+ int i;
+ int len;
argv_size = 1;
va_start (ap, first_arg);
@@ -62,8 +65,18 @@
rpos = arg;
while ('\0' != *rpos)
{
- if (' ' == *rpos)
+
+ if ('"' == *rpos)
{
+ if(quote_on == 1)
+ quote_on =0;
+ else
+ quote_on =1;
+
+ }
+
+ if ((' ' == *rpos)&&(quote_on == 0))
+ {
if (last != NULL)
argv_size++;
last = NULL;
@@ -83,6 +96,7 @@
/* *INDENT-ON* */
va_end (ap);
+ quote_on =0;
argv = GNUNET_malloc (argv_size * sizeof (char *));
argv_size = 0;
va_start (ap, first_arg);
@@ -96,8 +110,18 @@
pos = cp;
while ('\0' != *pos)
{
- if (' ' == *pos)
+
+ if ('"' == *pos)
{
+ if(quote_on == 1)
+ quote_on =0;
+ else
+ quote_on =1;
+
+ }
+
+ if ((' ' == *pos)&&(quote_on == 0))
+ {
*pos = '\0';
if (last != NULL)
argv[argv_size++] = GNUNET_strdup (last);
@@ -121,7 +145,19 @@
/* *INDENT-ON* */
va_end (ap);
argv[argv_size] = NULL;
- binary_path = argv[0];
+
+
+ for(i = 0; i < argv_size; i++){
+ len = strlen(argv[i]);
+ if((argv[i][0] == '"')&& (argv[i][len-1] == '"'))
+ {
+ argv[i][len-1] = '\0';
+ memmove(argv[i], argv[i]+1, strlen(argv[i]));
+
+ }
+ }
+ binary_path = argv[0];
+
proc = GNUNET_OS_start_process_v (pipe_control, std_inheritance, lsocks,
binary_path, argv);
while (argv_size > 0)
Index: src/arm/gnunet-service-arm.c
===================================================================
--- src/arm/gnunet-service-arm.c (revision 30501)
+++ src/arm/gnunet-service-arm.c (working copy)
@@ -414,6 +414,8 @@
SOCKTYPE *lsocks;
unsigned int ls;
char *binary;
+ char *quotedbinary;
+ int binaryLen;
/* calculate listen socket list */
lsocks = NULL;
@@ -486,6 +488,16 @@
"Starting service `%s' using binary `%s' and configuration `%s'\n",
sl->name, sl->binary, sl->config);
binary = GNUNET_OS_get_libexec_binary_path (sl->binary);
+
+ //Surround in quotes
+ binaryLen = strlen(binary);
+ quotedbinary = GNUNET_malloc ((binaryLen + 3) * sizeof (char *));
+ quotedbinary[0] = '"';
+ strcpy("edbinary[1],binary);
+ quotedbinary[binaryLen + 1] = '"';
+ quotedbinary[binaryLen + 2] = '\0';
+
+
GNUNET_assert (NULL == sl->proc);
if (GNUNET_YES == use_debug)
{
@@ -492,12 +504,12 @@
if (NULL == sl->config)
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary, "-L",
+ lsocks, loprefix, quotedbinary, "-L",
"DEBUG", options, NULL);
else
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary, "-c", sl->config, "-L",
+ lsocks, loprefix, quotedbinary, "-c", sl->config, "-L",
"DEBUG", options, NULL);
}
else
@@ -505,15 +517,16 @@
if (NULL == sl->config)
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary,
+ lsocks, loprefix, quotedbinary,
options, NULL);
else
sl->proc =
do_start_process (sl->pipe_control, GNUNET_OS_INHERIT_STD_OUT_AND_ERR,
- lsocks, loprefix, binary, "-c", sl->config,
+ lsocks, loprefix, quotedbinary, "-c", sl->config,
options, NULL);
}
GNUNET_free (binary);
+ GNUNET_free (quotedbinary);
if (sl->proc == NULL)
{
GNUNET_log (GNUNET_ERROR_TYPE_ERROR, _("Failed to start service `%s'\n"),
| ||||
|
|
Added a new patch that support argument quoting |
|
|
As I said on IRC, this would break things like setting PREFIX = valgrind --tool=memcheck in the configuration (which we do a lot for debugging). I understand that you want to fix "Program Files\" on W32, but I don't think this is quite what we might want (but I didn't have time to look into this in detail yet). |
|
|
Grothoff, could you check the latest patch ? It would support the prefix and surrounding arguments in quotes. |
|
|
New patch. Incrementing a pointer and deleting later is illegal. |
|
|
With that patch, I get: Nov 08 13:24:32-075794 test-arm-api-6095 ERROR Assertion failed at test_arm_api.c:179. (other tests in ARM even hang). Did you test this? |
|
|
Fixed a few issues with the patch, committed as SVN 30885. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-11-05 05:30 | bratao | New Issue | |
| 2013-11-05 05:30 | bratao | File Added: do_start_process.c.patch | |
| 2013-11-05 20:26 | bratao | Note Added: 0007590 | |
| 2013-11-05 20:51 | bratao | File Added: do_start_process.V2.patch | |
| 2013-11-05 20:52 | Christian Grothoff | Note Added: 0007591 | |
| 2013-11-05 20:57 | bratao | Note Added: 0007592 | |
| 2013-11-05 22:54 | bratao | File Added: do_start_process.V3.patch | |
| 2013-11-05 22:55 | bratao | Note Added: 0007593 | |
| 2013-11-08 13:26 | Christian Grothoff | Note Added: 0007611 | |
| 2013-11-08 13:27 | Christian Grothoff | Status | new => feedback |
| 2013-11-26 23:39 | Christian Grothoff | Assigned To | => Christian Grothoff |
| 2013-11-26 23:39 | Christian Grothoff | Status | feedback => assigned |
| 2013-11-26 23:39 | Christian Grothoff | Note Added: 0007714 | |
| 2013-11-26 23:39 | Christian Grothoff | Status | assigned => resolved |
| 2013-11-26 23:39 | Christian Grothoff | Fixed in Version | => 0.10.0 |
| 2013-11-26 23:39 | Christian Grothoff | Resolution | open => fixed |
| 2013-11-26 23:39 | Christian Grothoff | Target Version | => 0.10.0 |
| 2013-12-24 20:54 | Christian Grothoff | Status | resolved => closed |
