[gnuastro-commits] master bfc00b75: Library (checkset.h): general function to easily replace system()

[Mohammad Akhlaghi]

branch: master
commit bfc00b757b3be6a1c91c366c71c9648bd888298f
Author: Mohammad Akhlaghi <mohammad@akhlaghi.org>
Commit: Mohammad Akhlaghi <mohammad@akhlaghi.org>

    Library (checkset.h): general function to easily replace system()
    
    Until now, Gnuastro's PDF library had its own low-level call to the
    'fork+execl' functions. But we do not want each Gnuastro component that
    needs to run commands to have to do this low-level operation. Furthermore,
    the 'execl' function took arguments on the shell as a variable length
    argument list; again, making it hard to generalize.
    
    With this commit, a new internal Gnuastro library function
    ('gal_checkset_exec') has been added for this job: it will take a simple
    linked list of arguments that should be executed, internally build an
    array, then run 'execv' after forking a new process.
---
 bootstrap.conf                   |  1 +
 lib/checkset.c                   | 55 ++++++++++++++++++++++++++++++++++++++++
 lib/gnuastro-internal/checkset.h |  6 +++++
 lib/pdf.c                        | 45 +++++++++++++++-----------------
 lib/pointer.c                    |  3 ++-
 lib/type.c                       | 22 ++++++++--------
 6 files changed, 95 insertions(+), 37 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 540ddb77..0bb79236 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -203,6 +203,7 @@ gnulib_modules="
     error
     nproc
     stdio
+    execv
     select
     stdint
     strtod
diff --git a/lib/checkset.c b/lib/checkset.c
index 567aaa9f..7af8fb82 100644
--- a/lib/checkset.c
+++ b/lib/checkset.c
@@ -29,10 +29,12 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <sys/wait.h>
 #include <sys/stat.h>
 #include <sys/fcntl.h>
 
 #include <gnuastro/data.h>
+#include <gnuastro/pointer.h>
 
 #include <gnuastro-internal/timing.h>
 #include <gnuastro-internal/checkset.h>
@@ -507,6 +509,59 @@ gal_checkset_noprefix_isequal(char *string, char *prefix,
 
 
 
+
+/**************************************************************/
+/**********               Run commands             ************/
+/**************************************************************/
+/* We are not using the 'system()' command because that call '/bin/sh',
+   which may not always be usable within the running environment; see
+   https://savannah.nongnu.org/bugs/?65677. */
+int
+gal_checkset_exec(char *executable_abs_address, gal_list_str_t *args)
+{
+  pid_t pid;
+  char **argv;
+  int childstat=0;
+  gal_list_str_t *tmp;
+  size_t i, numargs=gal_list_str_number(args);
+
+  /* Allocate the array  */
+  argv=gal_pointer_allocate(GAL_TYPE_STRING, numargs+1, 0,
+                            __func__, "argv");
+
+  /* Fill the argument list array. */
+  tmp=args;
+  for(i=0;i<numargs;++i){ argv[i]=tmp->v; tmp=tmp->next; }
+  argv[numargs]=NULL;
+
+  /* Fork a new process to run 'exec'. */
+  pid=fork();
+  if(pid<0) error(EXIT_FAILURE, 0, "%s: could not build fork", __func__);
+  if(pid==0) execv(executable_abs_address, argv);   /* Child.  */
+  else       waitpid(pid, &childstat, 0);           /* Parent. */
+
+  /* Return the status of the child. */
+  free(argv);
+  return childstat;
+}
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
 /**************************************************************/
 /********** Set file names and check if they exist ************/
 /**************************************************************/
diff --git a/lib/gnuastro-internal/checkset.h b/lib/gnuastro-internal/checkset.h
index 09cfda3c..8dc32e52 100644
--- a/lib/gnuastro-internal/checkset.h
+++ b/lib/gnuastro-internal/checkset.h
@@ -104,6 +104,12 @@ int
 gal_checkset_noprefix_isequal(char *string, char *prefix,
                               const char *tocompare);
 
+/**************************************************************/
+/**********               Run commands             ************/
+/**************************************************************/
+int
+gal_checkset_exec(char *executable_abs_address, gal_list_str_t *args);
+
 
 /**************************************************************/
 /********** Set file names and check if they exist ************/
diff --git a/lib/pdf.c b/lib/pdf.c
index 497cafbf..030fec98 100644
--- a/lib/pdf.c
+++ b/lib/pdf.c
@@ -28,11 +28,11 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
-#include <sys/wait.h>
 
 #include <gnuastro/eps.h>
 #include <gnuastro/pdf.h>
 #include <gnuastro/jpeg.h>
+#include <gnuastro/pointer.h>
 
 #include <gnuastro-internal/checkset.h>
 
@@ -104,9 +104,9 @@ gal_pdf_write(gal_data_t *in, char *filename, float 
widthincm,
               uint32_t borderwidth, uint8_t bordercolor,
               int dontoptimize, gal_data_t *marks)
 {
-  pid_t pid;
-  int childstat=0;
+  int execstat=0;
   size_t w_h_in_pt[2];
+  gal_list_str_t *command=NULL;
   char *device, *devwp, *devhp, *devopt;
   char *epsname=gal_checkset_malloc_cat(filename, ".ps");
 
@@ -132,42 +132,37 @@ gal_pdf_write(gal_data_t *in, char *filename, float 
widthincm,
     error(EXIT_FAILURE, 0, "%s: asprintf allocation error", __func__);
 
   /* Run Ghostscript (if the command changes, also change the command in
-     the error message). We are not using the 'system()' command because
-     that call '/bin/sh', which may not always be usable within the running
-     environment; see https://savannah.nongnu.org/bugs/?65677.
-
-     For a very nice summary on fork/execl, see the first answer in the
-     link below.
-     
https://stackoverflow.com/questions/4204915/please-explain-the-exec-function-and-its-family
-
-     In summary: the child gets 'pid=0' and the parent gets the process ID
-     of the child. The parent then waits for the child to finish and
-     continues. */
-  pid=fork();
-  if(pid<0) error(EXIT_FAILURE, 0, "%s: could not build fork", __func__);
-  if(pid==0)
-    execl(PATH_GHOSTSCRIPT, "gs", "-q", "-o", filename, devopt,
-          devwp, devhp, "-dPDFFitPage", epsname, NULL);
-  else waitpid(pid, &childstat, 0);
-  if(childstat)
+     the error message).  */
+  gal_list_str_add(&command, PATH_GHOSTSCRIPT, 0);
+  gal_list_str_add(&command, "-q", 0);
+  gal_list_str_add(&command, "-o", 0);
+  gal_list_str_add(&command, filename, 0);
+  gal_list_str_add(&command, devopt, 0);
+  gal_list_str_add(&command, devwp, 0);
+  gal_list_str_add(&command, devhp, 0);
+  gal_list_str_add(&command, "-dPDFFitPage", 0);
+  gal_list_str_add(&command, epsname, 0);
+  gal_list_str_reverse(&command);
+  execstat=gal_checkset_exec(PATH_GHOSTSCRIPT, command);
+  if(execstat)
     error(EXIT_FAILURE, 0, "the Ghostscript command (printed at the "
           "end of this message) to convert/compile the EPS file (made "
           "by Gnuastro) to PDF was not successful (Ghostscript returned "
           "with status %d; its error message is shown above)! The EPS "
           "file ('%s') is left if you want to convert it through any "
           "other means (for example the 'epspdf' program). The "
-          "Ghostscript command was: %s -q -o %s %s %s %s -dPDFFitPage "
-          "%s", childstat, epsname, PATH_GHOSTSCRIPT, filename, devopt,
-          devwp, devhp, epsname);
+          "Ghostscript command was: %s", execstat, epsname,
+          gal_list_str_cat(command, ' '));
 
   /* Delete the EPS file. */
   errno=0;
   if(unlink(epsname))
     error(EXIT_FAILURE, errno, "%s", epsname);
 
-  /* Clean up. */
+  /* Clean up the command list and delete the EPS file. */
   free(devhp);
   free(devwp);
   free(devopt);
   free(epsname);
+  gal_list_str_free(command, 0);
 }
diff --git a/lib/pointer.c b/lib/pointer.c
index b7d5bb3c..8c91b389 100644
--- a/lib/pointer.c
+++ b/lib/pointer.c
@@ -94,7 +94,8 @@ gal_pointer_allocate(uint8_t type, size_t size, int clear,
               size * gal_type_sizeof(type), varname);
       else
         error(EXIT_FAILURE, errno, "%s: %zu bytes couldn't be allocated",
-              funcname ? funcname : __func__, size * gal_type_sizeof(type));
+              funcname ? funcname : __func__,
+              size * gal_type_sizeof(type));
     }
 
   return array;
diff --git a/lib/type.c b/lib/type.c
index 08b55578..1b4b0c26 100644
--- a/lib/type.c
+++ b/lib/type.c
@@ -72,26 +72,26 @@ gal_type_sizeof(uint8_t type)
 
     case GAL_TYPE_FLOAT32:
       if( sizeof (float) != 4 )
-        error(EXIT_FAILURE, 0, "%s: 'float' is not 32 bits on this machine",
-              __func__);
+        error(EXIT_FAILURE, 0, "%s: 'float' is not 32 bits on this "
+              "machine", __func__);
       return sizeof (float);
 
     case GAL_TYPE_FLOAT64:
       if( sizeof (double) != 8 )
-        error(EXIT_FAILURE, 0, "%s: 'double' is not 64 bits on this machine",
-              __func__);
+        error(EXIT_FAILURE, 0, "%s: 'double' is not 64 bits on this "
+              "machine", __func__);
       return sizeof (double);
 
     case GAL_TYPE_COMPLEX32:
       if( sizeof (float) != 4 )
-        error(EXIT_FAILURE, 0, "%s: 'float' is not 32 bits on this machine",
-              __func__);
+        error(EXIT_FAILURE, 0, "%s: 'float' is not 32 bits on this "
+              "machine", __func__);
       return sizeof (gsl_complex_float);
 
     case GAL_TYPE_COMPLEX64:
       if( sizeof (double) != 8 )
-        error(EXIT_FAILURE, 0, "%s: 'double' is not 64 bits on this machine",
-              __func__);
+        error(EXIT_FAILURE, 0, "%s: 'double' is not 64 bits on this "
+              "machine", __func__);
       return sizeof (gsl_complex);
 
     case GAL_TYPE_STRING:
@@ -102,9 +102,9 @@ gal_type_sizeof(uint8_t type)
             __func__, type);
     }
 
-  error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at %s so we can find "
-        "the cause of the problem. Control should not have reached the end of "
-        "this function", __func__, PACKAGE_BUGREPORT);
+  error(EXIT_FAILURE, 0, "%s: a bug! Please contact us at %s so we can "
+        "find the cause of the problem. Control should not have reached "
+        "the end of this function", __func__, PACKAGE_BUGREPORT);
   return -1;
 }