[groff] 02/05: [libgroff]: Validate DESC, font descriptions more.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit ee0942bdecdef59b202d84dc2a754d04288c9abd
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Tue Nov 2 18:23:09 2021 +1100

    [libgroff]: Validate DESC, font descriptions more.
    
    [libgroff]: Do more device and font description file validation, resolve
    an assertion failure arising from a negative declared device resolution,
    and correct a documentation error.
    
    * src/libs/libgroff/font.cpp (font::load): Include more information
      about invalid input in diagnostic messages.
      - When a kern pair's amount is missing or invalid, report the name of
        the kern pair.
      - When someone tries to declare the first entry in the charset section
        as an alias, report the glyph name.
      - Identify the token for the unnamed character if an attempt is made
        to alias it.
      - When an out-of-range character type is applied to a glyph, name the
        glyph.
    
      (font::load_desc): Same.
      - Drop redundant zero initialization of `res`.
      - Check all directives that take basic units for positive values,
        adding `res`, `unitwidth`, `paperwidth`, and `paperlength`.  Update
        this diagnostic to demand positive, not nonnegative, values.
      - When the font count is long in a `fonts` directive, report how many
        font names were declared (and thus expected).  (If the count is
        short, the next line is read for a font name, like 'tcommand' in our
        devutf8/DESC.)
      - When interpreting a `papersize` directive, throw an error and return
        false if `res` has not yet been encountered, since it is used in
        subsequent computations.
      - When a paper format cannot be determined, report the original
        declared value from the DESC file.  Use `strdup()` to save it since
        it gets clobbered by the resolving process.  `free()` the saved
        string when we're done, regardless of error condition.
    
    * doc/groff.texi (Device and Font Files):
    * man/groff_font.5.man (DESC file format): Document additional exception
      to order-indifference of directives: (at least one) `res` must precede
      `papersize`.
    
    Fixes <https://savannah.gnu.org/bugs/?61414>.
---
 ChangeLog                  | 42 ++++++++++++++++++++++++++++++++++++++++++
 doc/groff.texi             |  9 ++++++---
 man/groff_font.5.man       | 30 ++++++++++++++++++++----------
 src/libs/libgroff/font.cpp | 42 ++++++++++++++++++++++++++++--------------
 4 files changed, 96 insertions(+), 27 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e664c97..c53e2c1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,45 @@
+2021-11-02  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       [libgroff]: Do more device and font description file validation,
+       resolve an assertion failure arising from a negative declared
+       device resolution, and correct a documentation error.
+
+       * src/libs/libgroff/font.cpp (font::load): Include more
+       information about invalid input in diagnostic messages.
+         - When a kern pair's amount is missing or invalid, report the
+           name of the kern pair.
+         - When someone tries to declare the first entry in the charset
+           section as an alias, report the glyph name.
+         - Identify the token for the unnamed character if an attempt
+           is made to alias it.
+         - When an out-of-range character type is applied to a glyph,
+           name the glyph.
+       (font::load_desc): Same.
+         - Drop redundant zero initialization of `res`.
+         - Check all directives that take basic units for positive
+           values, adding `res`, `unitwidth`, `paperwidth`, and
+           `paperlength`.  Update this diagnostic to demand positive,
+           not nonnegative, values.
+         - When the font count is long in a `fonts` directive, report
+           how many font names were declared (and thus expected).  (If
+           the count is short, the next line is read for a font name,
+           like 'tcommand' in our devutf8/DESC.)
+         - When interpreting a `papersize` directive, throw an error
+           and return false if `res` has not yet been encountered,
+           since it is used in subsequent computations.
+         - When a paper format cannot be determined, report the
+           original declared value from the DESC file.  Use `strdup()`
+           to save it since it gets clobbered by the resolving process.
+           `free()` the saved string when we're done, regardless of
+           error condition.
+
+       * doc/groff.texi (Device and Font Files):
+       * man/groff_font.5.man (DESC file format): Document additional
+       exception to order-indifference of directives: (at least one)
+       `res` must precede `papersize`.
+
+       Fixes <https://savannah.gnu.org/bugs/?61414>.
+
 2021-11-01  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        [man]: Try to minimize the number of times a URI is broken when
diff --git a/doc/groff.texi b/doc/groff.texi
index 2d30c1c..955f491 100644
--- a/doc/groff.texi
+++ b/doc/groff.texi
@@ -17936,9 +17936,12 @@ styles (@file{R}, @file{I}, @file{B}, and @file{BI}, 
respectively).
 @pindex DESC@r{ file format}
 
 The @file{DESC} file contains a series of directives; each begins a
-line.  Except for the @code{charset} directive, which must come last (if
-at all), their order is not important.  If a directive name is
-duplicated, however, later entries in the file override previous ones.
+line.  Their order is not important, with two exceptions: (1) the
+@code{res} directive must precede any @code{papersize} directive; and
+(2) the @code{charset} directive must come last (if at all).  If a
+directive name is repeated, later entries in the file override previous
+ones (except that the paper dimensions are computed based on the
+@code{res} directive last seen when @code{papersize} is encountered).
 @cindex comments in device description files
 @cindex device description files, comments
 @kindex #
diff --git a/man/groff_font.5.man b/man/groff_font.5.man
index 0b15d88..33f880c 100644
--- a/man/groff_font.5.man
+++ b/man/groff_font.5.man
@@ -101,16 +101,26 @@ The
 file contains a series of directives;
 each begins a line.
 .
-Except for the
+Their order is not important,
+with two exceptions:
+(1) the
+.B res
+directive must precede any
+.B \%papersize
+directive;
+and
+(2) the
 .B charset
-directive,
-which must come last
-(if at all),
-their order is not important.
+directive must come last
+(if at all).
 .
-If a directive name is duplicated,
-however,
-later entries in the file override previous ones.
+If a directive name is repeated,
+later entries in the file override previous ones
+(except that the paper dimensions are computed based on the
+.B res
+directive last seen when
+.B \%papersize
+is encountered).
 .
 Comments start with the
 .RB \[lq] # \[rq]
@@ -179,7 +189,7 @@ The vertical dimension of the physical output medium is
 units
 (deprecated:
 use
-.B papersize
+.B \%papersize
 instead).
 .
 .
@@ -271,7 +281,7 @@ The horizontal dimension of the physical output medium is
 units
 (deprecated:
 use
-.B papersize
+.B \%papersize
 instead).
 .
 .
diff --git a/src/libs/libgroff/font.cpp b/src/libs/libgroff/font.cpp
index 9f57847..3ade71c 100644
--- a/src/libs/libgroff/font.cpp
+++ b/src/libs/libgroff/font.cpp
@@ -885,12 +885,13 @@ bool font::load(bool load_header_only)
        }
        p = strtok(0, WS);
        if (0 == p) {
-         t.error("missing kern amount");
+         t.error("missing kern amount for kern pair '%1 %2'", c1, c2);
          return false;
        }
        int n;
        if (sscanf(p, "%d", &n) != 1) {
-         t.error("invalid kern amount '%1'", p);
+         t.error("invalid kern amount '%1' for kern pair '%2 %3'", p,
+                 c1, c2);
          return false;
        }
        glyph *g1 = name_to_glyph(c1);
@@ -917,12 +918,12 @@ bool font::load(bool load_header_only)
        }
        if (p[0] == '"') {
          if (last_glyph == 0) {
-           t.error("first entry '%1' in 'charset' subsection is an"
-                   " alias", nm);
+           t.error("the first entry ('%1') in 'charset' subsection"
+                   " cannot be an alias", nm);
            return false;
          }
          if (strcmp(nm, "---") == 0) {
-           t.error("an unnamed character cannot be an alias");
+           t.error("an unnamed character ('---') cannot be an alias");
            return false;
          }
          glyph *g = name_to_glyph(nm);
@@ -942,7 +943,7 @@ bool font::load(bool load_header_only)
                              &metric.pre_math_space,
                              &metric.subscript_correction);
          if (nparms < 1) {
-           t.error("invalid width for '%1'", nm);
+           t.error("missing or invalid width for glyph '%1'", nm);
            return false;
          }
          p = strtok(0, WS);
@@ -956,7 +957,8 @@ bool font::load(bool load_header_only)
            return false;
          }
          if (type < 0 || type > 255) {
-           t.error("character type '%1' out of range", type);
+           t.error("character type '%1' out of range for '%2'", type,
+                   nm);
            return false;
          }
          metric.type = type;
@@ -1053,7 +1055,6 @@ bool font::load_desc()
   if ((fp = open_file("DESC", &path)) == 0)
     return false;
   text_file t(fp, path);
-  res = 0;
   while (t.next_line()) {
     char *p = strtok(t.buf, WS);
     assert(p != 0);
@@ -1074,12 +1075,16 @@ bool font::load_desc()
        t.error("'%1' directive given invalid number '%2'", p, q);
        return false;
       }
-      if ((strcmp(p, "sizescale") == 0
-         || strcmp(p, "hor") == 0
-         || strcmp(p, "vert") == 0)
+      if ((strcmp(p, "res") == 0
+          || strcmp(p, "hor") == 0
+          || strcmp(p, "vert") == 0
+          || strcmp(p, "unitwidth") == 0
+          || strcmp(p, "paperwidth") == 0
+          || strcmp(p, "paperlength") == 0
+          ||  strcmp(p, "sizescale") == 0)
          && val < 1) {
        t.error("expected argument to '%1' directive to be a"
-               " nonnegative number, got '%2'", p, val);
+               " positive number, got '%2'", p, val);
        return false;
       }
       *(table[idx-1].ptr) = val;
@@ -1121,18 +1126,25 @@ bool font::load_desc()
       }
       p = strtok(0, WS);
       if (p != 0) {
-       t.error("font count does not match number of fonts");
+       t.error("font count does not match declared number of fonts"
+               " ('%1')", nfonts);
        return false;
       }
       font_name_table[nfonts] = 0;
     }
     else if (strcmp("papersize", p) == 0) {
+      if (0 == res) {
+       t.error("'res' directive must precede 'papersize' in device"
+               " description file");
+       return false;
+      }
       p = strtok(0, WS);
       if (0 == p) {
        t.error("'papersize' directive requires an argument");
        return false;
       }
       bool found_paper = false;
+      char *savedp = strdup(p);
       while (p) {
        double unscaled_paperwidth, unscaled_paperlength;
        if (scan_papersize(p, &papersize, &unscaled_paperlength,
@@ -1145,9 +1157,11 @@ bool font::load_desc()
        p = strtok(0, WS);
       }
       if (!found_paper) {
-       t.error("bad paper size");
+       t.error("unable to determine a paper format from '%1'", savedp);
+       free(savedp);
        return false;
       }
+      free(savedp);
     }
     else if (strcmp("unscaled_charwidths", p) == 0)
       use_unscaled_charwidths = true;