Oops.

That was an artifact from the debugging I was doing for the patch and I forgot to remove it. Fixed in attached.

--Andrew Black

Martin Sebor wrote:
Andrew Black wrote:
Greetings all.

Attached is a patch that should allow the exec utility to execute 'complex' targets. These targets are executable names with an associated set of parameters. If a parameter to a target is %x, the options specified as the value of the -x command line switch are inserted at this position in the parameter string for the child process.


This looks good. Just one thing:

[...]
@@ -228,25 +349,26 @@
    @see process_results
 */
 static void
-run_target (char* target, char** childargv)
+run_target (char* target, char** argv)
[...]
-    printf ("%-18.18s ", target_name);
-    fflush (stdout);
+//     printf ("%-18.18s ", target_name);
+//     fflush (stdout);

I assume you didn't really mean to comment these out. Could you
please resend the patch?

Thanks
Martin
Index: util.cpp
===================================================================
--- util.cpp	(revision 427578)
+++ util.cpp	(working copy)
@@ -91,3 +91,28 @@
 
     return alloc;
 }
+
+/**
+   Wrapper for realloc(), providing return value checking.
+
+   @param source pointer to memory block to reallocate
+   @param size number of bytes of memory to allocate
+   @param file name of file calling method
+   @param line line number in file method was called from
+   @return (non-null) pointer to allocated bock of memory
+*/
+void*
+guarded_realloc (void* source, const size_t size, const char* const file, 
+                 const unsigned line)
+{
+    void* const alloc = realloc (source, size);
+
+    assert (0 != file);
+    assert (0 < size);
+
+    if ( 0 == alloc )
+        terminate ( 1, "malloc(%lu) at line %u of %s failed: %s\n", 
+                 (unsigned long)size, line, file, strerror (errno));
+
+    return alloc;
+}
Index: util.h
===================================================================
--- util.h	(revision 427578)
+++ util.h	(working copy)
@@ -47,4 +47,9 @@
 void* guarded_malloc (const size_t size, const char* const file, 
                       const unsigned line);
 
+#define RW_REALLOC(source, size)                 \
+    guarded_realloc(source, size, __FILE__, __LINE__)
+void* guarded_realloc (void* source, const size_t size, 
+                       const char* const file, const unsigned line);
+
 #endif   // RW_UTIL_H
Index: runall.cpp
===================================================================
--- runall.cpp	(revision 427578)
+++ runall.cpp	(working copy)
@@ -30,6 +30,7 @@
 #include <stdlib.h> /* for exit, free */
 #include <string.h> /* for str* */
 
+#include <ctype.h> /* for isspace */
 #include <unistd.h>
     /* for close, dup, exec, fork - remove when removing diff dependancy*/
 #include <sys/types.h>
@@ -47,6 +48,126 @@
 #endif   // ENOENT
 
 /**
+   Utility function to rework the argv array
+
+   target is either a 'bare' executable or a 'complex' executable.  A bare
+   executable is the path to an executable.  A complex executable is the
+   path to the executable, followed by a series of command line arguments.
+
+   If target is a bare executable, the arguments in the returned argv array
+   will be the target followed by the contents of the recieved argv array.
+   
+   If target is a complex executable, the arguments in the returned argv
+   array will be target, transformed into an array.  If a token in the 
+   argument string is '%x' (no quotes), the contents of the provided argv 
+   array will be inserted into the return array at that point.
+
+   @todo Figure out an escaping mechanism to allow '%x' to be passed to an
+   executable
+
+   @param target target to generate an argv array for
+   @param argv program wide argv array for child processes
+   @return processed argv array, usable in exec ()
+*/
+static char** const
+merge_argv (char* const target, char* const argv [])
+{
+    const size_t tlen = strlen (target);
+    char ** split = split_opt_string (target);
+    unsigned i, arg_count = 0, spl_count = 0, wld_count = 0;
+
+    assert (0 != target);
+    assert (0 != argv);
+
+    /* If the split of target only contains a single value, we may have a 
+     bare executable name */
+    if (!split [1]) {
+        /* Check if last character in the target is whitespace */
+        if (isspace (target [tlen])) {
+            /* If it is, we've got a complex executable with no arguments.
+               Therfore, return it as is.
+            */
+            return split;
+        } /* Otherwise, it's a bare executable, so append argv */
+
+        /* Figure out how many arguments we've got in argv*/
+        for (/* none */; argv [arg_count]; ++arg_count);
+
+        /* reallocate memory for copying them, extending the buffer */
+        split = (char**)RW_REALLOC (split, (arg_count + 1) * sizeof (char*));
+            
+        /* And copy the pointers */
+        for (i=0; i < arg_count; ++i)
+            split [i+1] = argv [i];
+
+        return split;
+    } /* Otherwise, it's a complex executable with 1 or more arguments */
+
+    /* Figure out how many instances of '%x' we've got */
+    for (spl_count = 1; split [spl_count]; ++spl_count) {
+        if ('%' == split [spl_count][0] && 'x' == split [spl_count][1] 
+            && '\0' == split [spl_count][2])
+            ++wld_count;
+    }
+
+    /* If we don't have any instances of '%x', we have a valid argv array, 
+       so return it as it is.
+    */
+    if (0 == wld_count)
+        return split;
+
+    /* Now we need to determine how large the argv array is */
+    for (/* none */; argv [arg_count]; ++arg_count);
+
+    if (0 == arg_count) {
+        /* We want to shrink the array, removing the '%x' terms*/
+        unsigned found = 0;
+        for (i = 1; i <= spl_count; ++i) {
+            if (split [i] && '%' == split [i][0] && 'x' == split [i][1] 
+                && '\0' == split [i][2])
+                ++found;
+            else
+                split [i - found] = split [i];
+        }
+    }
+    else if (1 == arg_count) {
+        /* We need to replace all the %x terms with argv [0] */
+        
+        for (i = 1; i < spl_count; ++i) {
+            if ('%' == split [i][0] && 'x' == split [i][1] 
+                && '\0' == split [i][2])
+                split [i] = argv [0];
+        }
+    }
+    else {
+        /* We need to resize the split array to hold the insertion (s) */
+        /* First, we realloc the array */
+        const unsigned new_len = spl_count + (arg_count - 1) * wld_count;
+        split = (char**)RW_REALLOC (split, sizeof (char**) * new_len);
+
+        /* Then we itterate backwards through the split array, transcribing 
+           elements as we go.  We have to go backwards, so we don't clobber
+           data in the process.
+        */
+        for (/* none */; wld_count; --spl_count) {
+            if (split [spl_count] && '%' == split [spl_count][0] 
+                && 'x' == split [spl_count][1] 
+                && '\0' == split [spl_count][2]) {
+                --wld_count;
+                for (i = arg_count; i; --i) {
+                    split [spl_count + (arg_count - 1) * wld_count + i - 1] = 
+                        argv [i - 1];
+                }
+            }
+            else
+                split [spl_count + (arg_count - 1) * wld_count] = 
+                    split [spl_count];
+        }
+    }
+    return split;
+}
+
+/**
    Preflight check to ensure that target is something that should be run.
 
    This method checks to see if target exists and is theoretically executable.
@@ -228,25 +349,26 @@
    @see process_results
 */
 static void
-run_target (char* target, char** childargv)
+run_target (char* target, char** argv)
 {
     struct exec_attrs status;
+    char** childargv = merge_argv (target, argv);
 
     assert (0 != target);
+    assert (0 != argv);
     assert (0 != childargv);
 
-    childargv [0] = target;
-    target_name = rw_basename (target);
+    target_name = rw_basename (childargv [0]);
 
     printf ("%-18.18s ", target_name);
     fflush (stdout);
 
-    if (!check_target_ok (target))
+    if (!check_target_ok (childargv [0]))
         return;
 
     status = exec_file (childargv);
 
-    process_results (target, &status);
+    process_results (childargv [0], &status);
 }
 
 /**
@@ -281,7 +403,7 @@
 
     if (0 < argc) {
         int i;
-        char** childargv = split_child_opts ();
+        char** childargv = split_opt_string (exe_opts);
 
         assert (0 != childargv);
         puts ("NAME               STATUS ASSRTS FAILED PERCNT");
@@ -290,8 +412,8 @@
             run_target (argv [i], childargv);
         }
 
-        if (childargv [1])
-            free (childargv [1]);
+        if (childargv [0])
+            free (childargv [0]);
         free (childargv);
     }
 
Index: cmdopt.cpp
===================================================================
--- cmdopt.cpp	(revision 427578)
+++ cmdopt.cpp	(working copy)
@@ -152,7 +282,7 @@
 }
 
 /**
-   Translates exe_opts into an array that can be passed to exec ().
+   Translates opts into an array that can be passed to exec().
 
    This method malloc ()s two blocks of memory.  The first block is the 
    generated argv index array.  This is the return value of this method.  The 
@@ -162,11 +292,11 @@
 
    @warning this logic is UTF-8 unsafe
    @warning I/O redirection command piping isn't supported in the parse logic
+   @param opts option string to split
    @return the parsed argv array
-   @see exe_opts
 */
 char**
-split_child_opts ()
+split_opt_string (const char* const opts)
 {
     char in_quote = 0;
     int in_escape = 0;
@@ -174,30 +304,30 @@
     const char *pos;
     char *target, *last;
     char **table_pos, **argv;
-    const size_t optlen = strlen (exe_opts);
+    const size_t optlen = strlen (opts);
 
-    assert (0 != exe_opts);
+    assert (0 != opts);
 
     if (0 == optlen) {
         /* Alloc a an index array to hold the program name  */
-        argv = (char**)RW_MALLOC (2 * sizeof (char*));
+        argv = (char**)RW_MALLOC (sizeof (char*));
 
         /* And tie the two together */
-        argv [1] = (char*)0;
+        argv [0] = (char*)0;
         return argv;
     }
 
-    table_pos = argv = (char**)RW_MALLOC ((optlen + 5) * sizeof (char*) / 2);
-    /* (strlen (exe_opts)+5)/2 is overkill for the most situations, but it is 
-       just large enough to handle the worst case scenario.  The worst case 
-       is a string similar to 'x y' or 'x y z', requiring array lengths of 4 
-       and 5 respectively.
+    table_pos = argv = (char**)RW_MALLOC ((optlen + 3) * sizeof (char*) / 2);
+    /* (strlen (opts)+3)/2 is overkill for the most situations, but it is just 
+       large enough to handle the worst case scenario.  The worst case is a 
+       string similar to 'x y' or 'x y z', requiring array lengths of 4 and 5 
+       respectively.
     */
 
-    last = target = argv [1] = (char*)RW_MALLOC (optlen + 1);
+    last = target = argv [0] = (char*)RW_MALLOC (optlen + 1);
 
     /* Transcribe the contents, handling escaping and splitting */
-    for (pos = exe_opts; *pos; ++pos) {
+    for (pos = opts; *pos; ++pos) {
         if (in_escape) {
             *(target++) = *pos;
             in_escape = 0;
@@ -210,7 +340,7 @@
             else {
                 if (in_token) {
                     *(target++) = '\0';
-                    *(++table_pos) = last;
+                    *(table_pos++) = last;
                     in_token = 0;
                 }
                 last = target;
@@ -242,9 +372,9 @@
 
     if (in_token) { /* close and record the final token */
         *(target++) = '\0';
-        *(++table_pos) = last;
+        *(table_pos++) = last;
     }
-    *(++table_pos) = (char*)0;/*And terminate the array*/
+    *table_pos = (char*)0;/*And terminate the array*/
 
     return argv;
 }
Index: cmdopt.h
===================================================================
--- cmdopt.h	(revision 427578)
+++ cmdopt.h	(working copy)
@@ -41,6 +42,6 @@
 eval_options (const int argc, char* const argv[]);
 
 char**
-split_child_opts();
+split_opt_string(const char* const opts);
 
 #endif   // RW_PARSE_OPTS_H

Reply via email to