Re: [Groff] Re: groff 1.17: pre-grohtml's unsafe temporary file handling & other improvements

[Gaius Mulley]

Hi Ralph,

> More great work -- where do you get the time!

the lull before the exam marking storm :-)

> Should `-D dir' be in here too?

yes it should, thanks..

> Sorry to just butt in and comment on your patch without lifting a
> finger myself.  Please don't think I'm ungrateful.

no need to apologise, eye balling code is very useful, and we can
use this to our advantage in the free software world!

Here is the patch again, with the bugs removed, I hope..

Thanks Gaius

--- groff-cvs/src/devices/grohtml/grohtml.man   Fri Apr 13 10:03:53 2001
+++ groff-html/src/devices/grohtml/grohtml.man  Mon Jun  4 23:40:07 2001
@@ -37,8 +37,10 @@
 .el .RB "[\ " "\\$1" "\ ]"
 ..
 .OP \-v?lrn
+.OP \-D dir
 .OP \-F dir
 .OP \-i resolution
+.OP \-I image stem
 .OP \-o image vertical offset
 .RI "[\ " files\|.\|.\|. "\ ]"
 .br
@@ -104,6 +106,19 @@
 select the resolution for all images.
 By default this is 80 pixels per inch.
 Example: -i100 indicates 100 pixels per inch.
+.TP
+.BI \-I stem
+determine the image stem name. If omitted grohtml uses
+.BR grohtml-<pid> .
+.TP
+.BI \-D dir
+informs
+.B grohtml
+to place all image files into directory,
+.BR dir .
+.I dir
+for all images.
+.BR grohtml-<pid> .
 .TP
 .B \-v
 Print the version number.
--- groff-cvs/src/devices/grohtml/post-html.cc  Mon Jun  4 11:17:31 2001
+++ groff-html/src/devices/grohtml/post-html.cc Tue Jun  5 19:48:02 2001
@@ -2879,7 +2879,7 @@
     { "version", no_argument, 0, 'v' },
     { NULL, 0, 0, 0 }
   };
-  while ((c = getopt_long(argc, argv, "o:i:F:vd?lrn", long_options, NULL))
+  while ((c = getopt_long(argc, argv, "o:i:I:D:F:vd?lrn", long_options, NULL))
         != EOF)
     switch(c) {
     case 'v':
@@ -2903,6 +2903,12 @@
     case 'i':
       /* handled by pre-html */
       break;
+    case 'I':
+      /* handled by pre-html */
+      break;
+    case 'D':
+      /* handled by pre-html */
+      break;
     case 'n':
       simple_anchors = TRUE;
       break;
@@ -2929,6 +2935,6 @@
 
 static void usage(FILE *stream)
 {
-  fprintf(stream, "usage: %s [-vld?n] [-F dir] [files ...]\n",
+  fprintf(stream, "usage: %s [-vld?n] [-D dir] [-I image_stem] [-F dir] [files 
...]\n",
          program_name);
 }
--- groff-cvs/src/preproc/html/pre-html.cc      Fri Apr 20 14:35:01 2001
+++ groff-html/src/preproc/html/pre-html.cc     Tue Jun  5 20:42:49 2001
@@ -58,6 +58,7 @@
 #define IMAGE_BOARDER_PIXELS       0
 #define MAX_WIDTH                  8   // inches
 #define INLINE_LEADER_CHAR      '\\'
+#define MAX_RETRIES             4096   // number of different page directory 
names to try before giving up
 
 #define TRANSPARENT  "-background \"#FFF\" -transparent \"#FFF\""
 
@@ -91,6 +92,7 @@
 static int   troff_arg      = 0;                // troff arg index
 static char *command_prefix = 0;                // optional prefix for some 
installations.
 static char *troff_command  = 0;
+static char *image_dir      = 0;                // user specified image 
directory
 #if defined(DEBUGGING)
 static int   debug          = FALSE;
 static char *troffFileName  = 0;                // output of pre-html output 
which is sent to troff -Tps
@@ -243,14 +245,49 @@
  *  makeFileName - creates the image filename template.
  */
 
-void makeFileName ()
+static void makeFileName (void)
 {
-  char buffer[8192];
+  char *buffer;
+  int   len=50; /* space for grohtml-<number>-%d and a possible / */
+
+  if (image_dir != NULL)
+    len =+ strlen(image_dir);
+  if (image_template != NULL)
+    len =+ strlen(image_template);
+
+  buffer = (char *)malloc(len);
+  if (buffer == NULL)
+    sys_fatal("malloc");
+
+  if (image_dir == NULL) {
+    image_dir = "";
+  } else if ((strlen(image_dir)>0) && (image_dir[strlen(image_dir)-1] != '/')) 
{
+    sprintf(buffer, "%s/", image_dir);
+    image_dir = strdup(buffer);
+    if (image_dir == NULL)
+      sys_fatal("strdup");
+  }
+  
+  if (image_template == NULL)
+    sprintf(buffer, "%sgrohtml-%d", image_dir, (int)getpid());
+  else
+    sprintf(buffer, "%s%s", image_dir, image_template);
 
-  sprintf(buffer, "grohtml-%d", (int)getpid());
   strcat(buffer, "-%d");
-  image_template = (char *)malloc(strlen(buffer)+1);
-  strcpy(image_template, buffer);
+  image_template = strdup(buffer);
+  if (image_template == NULL)
+    sys_fatal("strdup");
+}
+
+/*
+ *  checkImageDir - checks to see whether the image directory is available.
+ */
+
+static void checkImageDir (void)
+{
+  if ((image_dir != NULL) && (strcmp(image_dir, "") != 0))
+    if (! ((mkdir(image_dir, 0700) == 0) || (errno == EEXIST)))
+      error("cannot create directory `%1'", image_dir);
 }
 
 /*
@@ -263,12 +300,12 @@
     /*
      *  emit image name and enable output
      */
-    writeString("\\O2\\O1\\O4\n");
+    writeString("\\O[2]\\O[1]\\O[4]\n");
   } else {
     /*
      *  postscript, therefore emit image boundaries
      */
-    writeString("\\O2\\O4\n");
+    writeString("\\O[2]\\O[4]\n");
   }
 }
 
@@ -283,8 +320,8 @@
 static void write_start_image (IMAGE_ALIGNMENT pos, int is_html)
 {
   if (pos == INLINE) {
-    writeString("\\O3\\O5'");
-    writeString(image_template); writeString(".png'");
+    writeString("\\O[3]\\O[5 ");
+    writeString(image_template); writeString(".png]");
   } else {
     writeString(".begin \\{\\\n");
     switch (pos) {
@@ -307,10 +344,10 @@
     writeString("\\}\n");
   }
   if (is_html) {
-    writeString("\\O0\n");
+    writeString("\\O[0]\n");
   } else {
     // reset min/max registers
-    writeString("\\O0\\O1\n");
+    writeString("\\O[0]\\O[1]\n");
   }
 }
 
@@ -574,12 +611,38 @@
  *  createAllPages - creates a set of images, one per page.
  */
 
-static void createAllPages (void)
+static int createAllPages (void)
 {
   char buffer[4096];
+  int retries = MAX_RETRIES;
+
+  imagePageStem = xtmptemplate("-page-");
+  strcpy(buffer, imagePageStem);
+
+  do {
+    if (mktemp(imagePageStem) == NULL) {
+      sys_fatal("mktemp");
+      return -1;
+    }
+    if (mkdir(imagePageStem, 0700) == 0) break;
+    if (errno == EEXIST) {
+      // directory already exists, try another name
+      retries--;
+      if (retries == 0) {
+       // time to give up
+       sys_fatal("mkdir");
+       return -1;
+      }
+    } else {
+      // another error, quit
+      sys_fatal("mkdir");
+      return -1;
+    }      
+    strcpy(imagePageStem, buffer);
+  } while (1);
 
   sprintf(buffer,
-         "echo showpage | gs -q -dSAFER -sDEVICE=%s -r%d -sOutputFile=%s%%d %s 
- > /dev/null 2>&1 \n",
+         "echo showpage | gs -q -dSAFER -sDEVICE=%s -r%d -sOutputFile=%s/%%d 
%s - > /dev/null 2>&1 \n",
          image_device,
          image_res,
          imagePageStem,
@@ -589,6 +652,7 @@
   fflush(stderr);
 #endif
   system(buffer);
+  return 0;
 }
 
 /*
@@ -602,9 +666,10 @@
   int  i=1;
 
   do {
-    sprintf(buffer, "%s%d", imagePageStem, i);
+    sprintf(buffer, "%s/%d", imagePageStem, i);
     i++;
-  } while (remove(buffer) == 0);
+  } while (unlink(buffer) == 0);
+  rmdir(imagePageStem);
 #endif
 }
 
@@ -648,7 +713,7 @@
     int  y2 = (image_res*vertical_offset/72)+max(i->Y1, 
i->Y2)*image_res/POSTSCRIPTRES+1*IMAGE_BOARDER_PIXELS;
 
     sprintf(buffer,
-           "pnmcut %d %d %d %d < %s%d | pnmtopng %s > %s \n",
+           "pnmcut %d %d %d %d < %s/%d | pnmtopng %s > %s \n",
            x1, y1, x2-x1+1, y2-y1+1,
            imagePageStem,
            i->pageNo,
@@ -987,13 +1052,15 @@
 
 void usage(FILE *stream)
 {
-  fprintf(stream, "usage: %s troffname [-P-o vertical_image_offset] [-P-i 
image_resolution] [troff flags] [files]\n", program_name);
+  fprintf(stream, "usage: %s troffname [-Iimage_name] [-Dimage_directory] 
[-P-o vertical_image_offset] [-P-i image_resolution] [troff flags] [files]\n", 
program_name);
   fprintf(stream, "    vertical_image_offset (default %d/72 of an inch)\n", 
vertical_offset);
   fprintf(stream, "    image_resolution (default %d) pixels per inch\n", 
image_res);
+  fprintf(stream, "    image_name is the name of the stem for all images 
(default is grohtml-<pid>)\n");
+  fprintf(stream, "    place all png files into image_directory\n");
 }
 
 /*
- *  scanArguments - scans for -P-i and -P-o arguments.
+ *  scanArguments - scans for -P-i, -P-o, -P-D and -P-I arguments.
  */
 
 int scanArguments (int argc, char **argv)
@@ -1001,7 +1068,11 @@
   int i=1;
 
   while (i<argc) {
-    if (strncmp(argv[i], "-i", 2) == 0) {
+    if (strncmp(argv[i], "-D", 2) == 0) {
+      image_dir = (char *)(argv[i]+2);
+    } else if (strncmp(argv[i], "-I", 2) == 0) {
+      image_template = (char *)(argv[i]+2);
+    } else if (strncmp(argv[i], "-i", 2) == 0) {
       image_res = atoi((char *)(argv[i]+2));
     } else if (strncmp(argv[i], "-o", 2) == 0) {
       vertical_offset = atoi((char *)(argv[i]+2));
@@ -1033,7 +1104,7 @@
  *  makeTempFiles - name the temporary files
  */
 
-static void makeTempFiles (void)
+static int makeTempFiles (void)
 {
 #if defined(DEBUGGING)
   psFileName     = "/tmp/prehtml-ps";
@@ -1042,10 +1113,21 @@
   troffFileName  = "/tmp/prehtml-troff";
   htmlFileName   = "/tmp/prehtml-html";
 #else
-  psFileName     = mktemp(xtmptemplate("-ps-"));
-  regionFileName = mktemp(xtmptemplate("-regions-"));
-  imagePageStem  = mktemp(xtmptemplate("-page-"));
+  int fd;
+
+  if ((fd = mkstemp(psFileName = xtmptemplate("-ps-"))) == -1) {
+    sys_fatal("mkstemp");
+    return -1;
+  }
+  close(fd);
+  if ((fd = mkstemp(regionFileName = xtmptemplate("-regions-"))) == -1) {
+    sys_fatal("mkstemp");
+    unlink(psFileName);
+    return -1;
+  }
+  close(fd);
 #endif
+  return 0;
 }
 
 /*
@@ -1055,8 +1137,8 @@
 static void removeTempFiles (void)
 {
 #if !defined(DEBUGGING)
-  remove(psFileName);
-  remove(regionFileName);
+  unlink(psFileName);
+  unlink(regionFileName);
 #endif
 }
 
@@ -1084,8 +1166,9 @@
   int ok=1;
 
   findPrefix();
-  makeFileName();
   i = scanArguments(argc, argv);
+  checkImageDir();
+  makeFileName();
   while (i < argc) {
     if (argv[i][0] != '-') {
       /* found source file */
@@ -1103,13 +1186,16 @@
   if (! found) {
     do_file("-");
   }
-  makeTempFiles();
+  if (makeTempFiles())
+    return 1;
   ok = inputFile.do_image(argc, argv);
   if (ok == 0) {
-    createAllPages();
-    generateImages(regionFileName);
-    ok = inputFile.do_html(argc, argv);
-    removeAllPages();
+    ok = createAllPages();
+    if (ok == 0) {
+      generateImages(regionFileName);
+      ok = inputFile.do_html(argc, argv);
+      removeAllPages();
+    }
   }
   removeTempFiles();
   return ok;
--- groff-cvs/src/roff/troff/input.cc   Mon Jun  4 11:17:50 2001
+++ groff-html/src/roff/troff/input.cc  Mon Jun  4 16:03:26 2001
@@ -135,6 +135,7 @@
 static request_or_macro *lookup_request(symbol);
 static int get_delim_number(units *, int);
 static int get_delim_number(units *, int, units);
+static symbol get_delim_file_name();
 static int get_line_arg(units *res, int si, charinfo **cp);
 static int read_size(int *);
 static symbol get_delim_name();
@@ -4084,6 +4085,66 @@
   }
 }
 
+static symbol get_delim_file_name()
+{
+  token start;
+  start.next();
+  if (start.eof()) {
+    error("end of input at start of delimited file name");
+    return NULL_SYMBOL;
+  }
+  if (start.newline()) {
+    error("can't delimit file name with a newline");
+    return NULL_SYMBOL;
+  }
+  int start_level = input_stack::get_level();
+  char abuf[ABUF_SIZE];
+  char *buf = abuf;
+  int buf_size = ABUF_SIZE;
+  int i = 0;
+  for (;;) {
+    if (i + 1 > buf_size) {
+      if (buf == abuf) {
+       buf = new char[ABUF_SIZE*2];
+       memcpy(buf, abuf, buf_size);
+       buf_size = ABUF_SIZE*2;
+      }
+      else {
+       char *old_buf = buf;
+       buf = new char[buf_size*2];
+       memcpy(buf, old_buf, buf_size);
+       buf_size *= 2;
+       a_delete old_buf;
+      }
+    }
+    tok.next();
+    if (tok.ch() == ']' && input_stack::get_level() == start_level)
+      break;
+
+    if ((buf[i] = tok.ch()) == 0) {
+      error("missing delimiter (got %1)", tok.description());
+      if (buf != abuf)
+       a_delete buf;
+      return NULL_SYMBOL;
+    }
+    i++;
+  }
+  buf[i] = '\0';
+  if (buf == abuf) {
+    if (i == 0) {
+      error("empty delimited file name");
+      return NULL_SYMBOL;
+    }
+    else
+      return symbol(buf);
+  }
+  else {
+    symbol s(buf);
+    a_delete buf;
+    return s;
+  }
+}
+
 // Implement \R
 
 static void do_register()
@@ -4324,6 +4385,13 @@
 {
   tok.next();
   int c = tok.ch();
+  if (c != '[') {
+    error("`\\O%1' is incorrect syntax, use `\\O[%1]'", char(c), char(c));
+    return 0;
+  }
+  tok.next();
+  c = tok.ch();
+  tok.next();
   switch (c) {
   case '0':
     if (begin_level == 1)
@@ -4344,14 +4412,19 @@
     begin_level--;
     break;
   case '5': {
-    symbol filename = get_delim_name();
+    symbol filename = get_delim_file_name();
+    tok.next();
+    if (filename.is_null()) {
+      error("\\O[5 requires a filename");
+      return 0;
+    }
     if (begin_level == 1)
       return new suppress_node(filename, 'i');
     return 0;
     break;
   }
   default:
-    error("`%1' is an invalid argument to \\O", char(c));
+    error("`%1' is an invalid argument to \\O[", char(c));
   }
   return 0;
 }
--- groff-cvs/tmac/www.tmac     Sat Apr 14 15:25:15 2001
+++ groff-html/tmac/www.tmac    Mon Jun  4 12:13:27 2001
@@ -159,19 +159,19 @@
 .         image \\$2 \\$1.png
 .         bp
 .         tl ''''
-\O0\O1
+\O[0]\O[1]
 .  \}
 .  if \\n[www-html] .begin \{
 .         image \\$2 \\$1.png
-\O0
+\O[0]
 .  \}
 ..
 .\"
 .\" HTML-IMAGE-END - terminates an image for html
 .\"
 .de HTML-IMAGE-END
-.  if r ps4html    \O2\O1\O4
-.  if \\n[www-html] \O2\O1\O4
+.  if r ps4html    \O[2]\O[1]\O[4]
+.  if \\n[www-html] \O[2]\O[1]\O[4]
 ..
 .nr png-no 0
 .\"