[groff] 03/08: [grohtml]: Fix Savannah #60981.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit 5e5cd25d4eec5481c836f8307c943a9043949647
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Wed Jul 28 16:51:41 2021 +1000

    [grohtml]: Fix Savannah #60981.
    
    Refactor device description file handling, make it more robust, and make
    this preprocessor's -F option work as documented, and to honor
    documented device description file semantics.
    
    * src/preproc/html/pre-html.cpp: Store partial filespecs of HTML and
      PostScript device files in constant strings.
    
      (get_resolution): Initialize `res` to zero.  Only `free()` the pointer
      `pathp` if `open_file()` succeeded (populating it).  Stop checking for
      valid `sscanf()` conversions of an integer to store in `res`, and
      don't return early.  Instead process the whole DESC file; per our
      Texinfo manual and groff_font(5), "Later entries in the file ...
      override previous values."
    
      (get_image_generator): Add new function, paralleling
      `get_resolution()`, replacing open-coded logic in `main()`.  Only the
      body of the `while` loop significantly differs.  Instead of using
      `sscanf`, process the input character by character matching the
      keyword and {1,} spaces or tabs after it.  Whatever follows in
      `linebuf` must be the program name.
    
      (imageList::createPage): Add `assert()` to cause failure if an empty
      image generator program gets this far, because it creates repeated
      messes if it does.  (Something isn't checking exit statuses.)
    
      (main): Condense `image_gen` collection to a function call, a null
      pointer check, and a fatal diagnostic.  Add a sanity check and a fatal
      diagnostic if the PostScript resolution is garbage.  (It's initialized
      to -1 and `get_resolution()` will return 0 if it doesn't find one.)
    
    Fixes <https://savannah.gnu.org/bugs/?60981>.
---
 ChangeLog                     | 34 ++++++++++++++++++++
 src/preproc/html/pre-html.cpp | 74 +++++++++++++++++++++++++++++++++----------
 2 files changed, 91 insertions(+), 17 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5fc0124..2436504 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,39 @@
 2021-07-28  G. Branden Robinson <g.branden.robinson@gmail.com>
 
+       [grohtml]: Fix Savannah #60981.  Refactor device description
+       file handling, make it more robust, and make this preprocessor's
+       -F option work as documented, and to honor documented device
+       description file semantics.
+
+       * src/preproc/html/pre-html.cpp: Store partial filespecs of HTML
+       and PostScript device files in constant strings.
+       (get_resolution): Initialize `res` to zero.  Only `free()` the
+       pointer `pathp` if `open_file()` succeeded (populating it).
+       Stop checking for valid `sscanf()` conversions of an integer to
+       store in `res`, and don't return early.  Instead process the
+       whole DESC file; per our Texinfo manual and groff_font(5),
+       "Later entries in the file ... override previous values."
+       (get_image_generator): Add new function, paralleling
+       `get_resolution()`, replacing open-coded logic in `main()`.
+       Only the body of the `while` loop significantly differs.
+       Instead of using `sscanf`, process the input character by
+       character matching the keyword and {1,} spaces or tabs
+       after it.  Whatever follows in `linebuf` must be the program
+       name.
+       (imageList::createPage): Add `assert()` to cause failure if an
+       empty image generator program gets this far, because it creates
+       repeated messes if it does.  (Something isn't checking exit
+       statuses.)
+       (main): Condense `image_gen` collection to a function call, a
+       null pointer check, and a fatal diagnostic.  Add a sanity check
+       and a fatal diagnostic if the PostScript resolution is garbage.
+       {It's initialized to -1 and `get_resolution()` will return 0 if
+       it doesn't find one.}
+
+       Fixes <https://savannah.gnu.org/bugs/?60981>.
+
+2021-07-28  G. Branden Robinson <g.branden.robinson@gmail.com>
+
        [grohtml]: Delete or rename `debug` symbols to avoid clash with
        new `debug` function in libgroff.
 
diff --git a/src/preproc/html/pre-html.cpp b/src/preproc/html/pre-html.cpp
index 5a005b5..7f12c1f 100644
--- a/src/preproc/html/pre-html.cpp
+++ b/src/preproc/html/pre-html.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 2000-2020 Free Software Foundation, Inc.
+/* Copyright (C) 2000-2021 Free Software Foundation, Inc.
  * Written by Gaius Mulley (gaius@glam.ac.uk).
  *
  * This file is part of groff.
@@ -217,6 +217,9 @@ static char *linebuf = NULL;                // for scanning 
devps/DESC
 static int linebufsize = 0;
 static const char *image_gen = NULL;    // the 'gs' program
 
+static const char devhtml_desc[] = "devhtml/DESC";
+static const char devps_desc[] = "devps/DESC";
+
 const char *const FONT_ENV_VAR = "GROFF_FONT_PATH";
 static search_path font_path(FONT_ENV_VAR, FONTPATH, 0, 0);
 static html_dialect dialect = html4;
@@ -287,27 +290,61 @@ int get_line(FILE *f)
 }
 
 /*
- *  get_resolution - Return the postscript resolution from devps/DESC.
+ *  get_resolution - Return the PostScript device resolution.
  */
 
 static unsigned int get_resolution(void)
 {
   char *pathp;
   FILE *f;
-  unsigned int res;
-  f = font_path.open_file("devps/DESC", &pathp);
+  unsigned int res = 0;
+  f = font_path.open_file(devps_desc, &pathp);
+  if (0 == f)
+    fatal("cannot open file '%1'", devps_desc);
   free(pathp);
-  if (f == 0)
-    fatal("can't open devps/DESC");
   while (get_line(f)) {
-    int n = sscanf(linebuf, "res %u", &res);
-    if (n >= 1) {
-      fclose(f);
-      return res;
+    (void) sscanf(linebuf, "res %u", &res);
+  }
+  fclose(f);
+  return res;
+}
+
+
+/*
+ *  get_image_generator - Return the declared program from the HTML
+ *                        device description.
+ */
+
+static char *get_image_generator(void)
+{
+  char *pathp;
+  FILE *f;
+  char *generator = 0;
+  const char keyword[] = "image_generator";
+  size_t keyword_len = strlen(keyword);
+  f = font_path.open_file(devhtml_desc, &pathp);
+  if (0 == f)
+    fatal("cannot open file '%1'", devhtml_desc);
+  free(pathp);
+  while (get_line(f)) {
+    char *cursor = linebuf;
+    size_t limit = strlen(linebuf);
+    char *end = linebuf + limit;
+    if (0 == (strncmp(linebuf, keyword, keyword_len))) {
+      cursor += keyword_len;
+      // At least one space or tab is required.
+      if(!(' ' == *cursor) || ('\t' == *cursor))
+       continue;
+      cursor++;
+      while((cursor < end) && (' ' == *cursor) || ('\t' == *cursor))
+       cursor++;
+      if (cursor == end)
+       continue;
+      generator = cursor;
     }
   }
-  fatal("can't find 'res' keyword in devps/DESC");
-  return 0;
+  fclose(f);
+  return generator;
 }
 
 /*
@@ -929,6 +966,7 @@ int imageList::createPage(int pageno)
     sys_fatal("make_message");
   html_system(s, 1);
 
+  assert(strlen(image_gen) > 0);
   s = make_message("echo showpage | "
                   "%s%s -q -dBATCH -dSAFER "
                   "-dDEVICEHEIGHTPOINTS=792 "
@@ -1784,12 +1822,14 @@ int main(int argc, char **argv)
 #endif /* CAPTURE_MODE */
   device = "html";
   i = scanArguments(argc, argv);
-  if (!font::load_desc())
-    fatal("cannot find devhtml/DESC exiting");
-  image_gen = font::image_generator;
-  if (image_gen == NULL || (strcmp(image_gen, "") == 0))
-    fatal("devhtml/DESC must set the image_generator field, exiting");
+  image_gen = strsave(get_image_generator());
+  if (0 == image_gen)
+    fatal("'image_generator' directive not found in file '%1'",
+         devhtml_desc);
   postscriptRes = get_resolution();
+  if (postscriptRes < 1) // TODO: what's a more sane minimum value?
+    fatal("'res' directive missing or invalid in file '%1'",
+         devps_desc);
   setupAntiAlias();
   checkImageDir();
   makeFileName();