Re: [PATCH] Quote filename passed to $EDITOR

On Tue, Jul 23, 2002 at 11:37:56AM -0500, cmpilato@collab.net wrote:
> > Why not use a replacement for system that doesn't run through a
> > shell in the first place, fork()/execve().
>
> We've been here. Let it die.

Ehh? This strikes me as a much better way to do it. Below is a patch
that should do this portably, though please don't apply yet - I think I
will factor the messiness into a new function and use it for other
invocations (diff?)

-- 
Scott Lamb
Index: ./subversion/clients/cmdline/util.c
===================================================================
--- ./subversion/clients/cmdline/util.c
+++ ./subversion/clients/cmdline/util.c	Tue Jul 23 12:46:07 2002
@@ -348,15 +348,18 @@
                          apr_pool_t *pool)
 {
   const char *editor = NULL;
-  const char *cmd;
   apr_file_t *tmp_file;
   const char *tmpfile_name;
   const char *contents_native, *tmpfile_native;
+  const char *args[3];
   apr_status_t apr_err, apr_err2;
   apr_size_t written;
   apr_finfo_t finfo_before, finfo_after;
   svn_error_t *err = SVN_NO_ERROR;
-  int sys_err;
+  apr_procattr_t *attr = NULL;
+  apr_proc_t proc;
+  int exitcode;
+  apr_exit_why_e exitwhy;
 
   /* Try to find an editor in the environment. */
   editor = getenv ("SVN_EDITOR");
@@ -416,14 +419,49 @@
     }
 
   /* Now, run the editor command line.  */
-  cmd = apr_psprintf (pool, "%s \"%s\"", editor, tmpfile_native);
-  sys_err = system (cmd);
-  if (sys_err != 0)
+  apr_err = apr_procattr_create(&attr, pool);
+  if (apr_err)
+    {
+      err =  svn_error_createf (apr_err, 0, NULL, pool,
+                                "apr_procattr_create failed");
+      goto cleanup;
+    }
+
+  apr_err = apr_procattr_cmdtype_set(attr, APR_PROGRAM_PATH);
+  if (apr_err)
+    {
+      err =  svn_error_createf (apr_err, 0, NULL, pool,
+                                "apr_procattr_cmdtype_set failed");
+      goto cleanup;
+    }
+
+  args[0] = editor;
+  args[1] = tmpfile_native;
+  args[2] = NULL;
+  apr_err = apr_proc_create(&proc, editor, args, NULL, attr, pool);
+  if (apr_err)
+    {
+      err =  svn_error_createf (apr_err, 0, NULL, pool,
+                                "apr_proc_create('%s %s') failed",
+                                editor, tmpfile_native);
+      goto cleanup;
+    }
+
+  while ((apr_err = apr_proc_wait(&proc, &exitcode, &exitwhy, APR_WAIT))
+         == APR_CHILD_NOTDONE) ;
+  if (apr_err != APR_CHILD_DONE)
+    {
+      err =  svn_error_createf (apr_err, 0, NULL, pool,
+                                "apr_proc_wait failed");
+      goto cleanup;
+    }
+
+  if (exitwhy != APR_PROC_EXIT
+      || (exitcode != 0 && exitcode != APR_ENOTIMPL))
     {
-      /* Extracting any meaning from sys_err is platform specific, so just
-         use the raw value. */
       err =  svn_error_createf (SVN_ERR_EXTERNAL_PROGRAM, 0, NULL, pool,
-                                "system('%s') returned %d", cmd, sys_err);
+                                "abnormal exit from '%s %s': %d",
+                                editor, tmpfile_native, exitcode);
       goto cleanup;
     }
   
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org