bugGNU roff - Bugs: bug #64155, specifying -fZD on command line...

 
 

bug #64155: specifying -fZD on command line generates warnings

Submitter:  Dave <barx>
Submitted:  Fri 05 May 2023 11:41:19 PM UTC
   
 
Category:  Font - others/general Severity:  2 - Minor
Item Group:  Warning/Suspicious behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.24.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 10 Jul 2023 08:34:09 AM UTC, comment #17: 


commit 39ffa368dc6a1de4c11cf3f4f5b8594d3c974173
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu May 25 05:35:25 2023 -0500

    [troff]: Validate a font family before using it.

    * src/roff/troff/env.cpp (is_family_valid): New function checks for all
      text styles (R, I, B, BI) and returns true only if the given family
      supports them all.

      (family_change): Call `is_family_valid()` on given argument.  If
      invalid, throw diagnostic and ignore `fam` request.

    * src/roff/troff/env.h (is_family_valid): Declare; make visible.

    * src/roff/troff/input.cpp (main): Call `is_family_valid()` on `-f`
      option argument.  Its invalidity is a fatal error.

    Fixes <https://savannah.gnu.org/bugs/?64155>.  Thanks to Dave Kemper for
    the report.

    Tested with:

    $ echo | ./build/test-groff -fZD -z
    troff: fatal error: 'ZD' is not a valid font family
    $ echo | ./build/test-groff -fH -z
    $ nl ./EXPERIMENTS/validate-family.groff
         1  .tm .fam=\n[.fam]
         2  .fam H
         3  .tm .fam=\n[.fam]
         4  .fam
         5  .tm .fam=\n[.fam]
         6  .fam ZD
         7  .fam BOGUS
         8  .tm .fam=\n[.fam]
         9  .fam
        10  .tm .fam=\n[.fam]
    $ ./build/test-groff ./EXPERIMENTS/validate-family.groff
    .fam=T
    .fam=H
    .fam=T
    troff:./EXPERIMENTS/validate-family.groff:6: error: 'ZD' is not a valid font family
    troff:./EXPERIMENTS/validate-family.groff:7: error: 'BOGUS' is not a valid font family
    .fam=T
    .fam=H


G. Branden Robinson <gbranden>
Group administrator
Thu 01 Jun 2023 02:50:39 PM UTC, comment #16: 

Fixed on my branch.

And oh, look, a bug number typo in the commit message.  Ah, but I can fix it when merging to master.


commit 1fea0c142fae929cce003d6e529e000687149735
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu May 25 05:35:25 2023 -0500

    [troff]: Validate a font family before using it.

    * src/roff/troff/env.cpp (is_family_valid): New function checks for all
      text styles (R, I, B, BI) and returns true only if the given family
      supports them all.

      (family_change): Call `is_family_valid()` on given argument.  If
      invalid, throw diagnostic and ignore `fam` request.

    * src/roff/troff/env.h (is_family_valid): Declare; make visible.

    * src/roff/troff/input.cpp (main): Call `is_family_valid()` on `-f`
      option argument.  Its invalidity is a fatal error.

    Fixes <https://savannah.gnu.org/bugs/?64115>.  Thanks to Dave
    Kemper for the report.

    Tested with:

    $ echo | ./build/test-groff -fZD -z
    troff: fatal error: 'ZD' is not a valid font family
    $ echo | ./build/test-groff -fH -z
    $ nl ./EXPERIMENTS/validate-family.groff
         1  .tm .fam=\n[.fam]
         2  .fam H
         3  .tm .fam=\n[.fam]
         4  .fam
         5  .tm .fam=\n[.fam]
         6  .fam ZD
         7  .fam BOGUS
         8  .tm .fam=\n[.fam]
         9  .fam
        10  .tm .fam=\n[.fam]
    $ ./build/test-groff ./EXPERIMENTS/validate-family.groff
    .fam=T
    .fam=H
    .fam=T
    troff:./EXPERIMENTS/validate-family.groff:6: error: 'ZD' is not a valid font family
    troff:./EXPERIMENTS/validate-family.groff:7: error: 'BOGUS' is not a valid font family
    .fam=T
    .fam=H


G. Branden Robinson <gbranden>
Group administrator
Thu 25 May 2023 03:31:05 PM UTC, comment #15: 

In my working copy of my private branch.  The commit ID will almost certainly change.


commit f8e966af2985f267ed1efd9c6eeb37e41adca959
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu May 25 05:35:25 2023 -0500

    [troff]: Validate a font family before using it.

    * src/roff/troff/env.cpp (is_family_valid): New function checks for all
      text styles (R, I, B, BI) and returns true only if the given family
      supports them all.

      (family_change): Call `is_family_valid()` on given argument.  If
      invalid, throw diagnostic and ignore `fam` request.

    * src/roff/troff/env.h (is_family_valid): Declare; make visible.

    * src/roff/troff/input.cpp (main): Call `is_family_valid()` on `-f`
      option argument.  Its invalidity is a fatal error.

    Fixes <https://savannah.gnu.org/bugs/?64115>.  Thanks to Dave
    Kemper for the report.

    Tested with:

    $ echo | ./build/test-groff -fZD -z
    troff: fatal error: 'ZD' is not a valid font family
    $ echo | ./build/test-groff -fH -z
    $ nl ./EXPERIMENTS/validate-family.groff
         1  .tm .fam=\n[.fam]
         2  .fam H
         3  .tm .fam=\n[.fam]
         4  .fam
         5  .tm .fam=\n[.fam]
         6  .fam ZD
         7  .fam BOGUS
         8  .tm .fam=\n[.fam]
         9  .fam
        10  .tm .fam=\n[.fam]
    $ ./build/test-groff ./EXPERIMENTS/validate-family.groff
    .fam=T
    .fam=H
    .fam=T
    troff:./EXPERIMENTS/validate-family.groff:6: error: 'ZD' is not a valid font family
    troff:./EXPERIMENTS/validate-family.groff:7: error: 'BOGUS' is not a valid font family
    .fam=T
    .fam=H


I did have to lose the bit of diagnostic about which style was missing, because the function needed to be callable from two different places and passing back that information promised to be a pain in the butt.  It would have meant adding a reference or pointer argument to `is_family_valid`, or introducing C++ exceptions to groff's exception-free code base.  Or something similarly grody, I expect.  It didn't seem worth the trouble at this time.

G. Branden Robinson <gbranden>
Group administrator
Thu 25 May 2023 11:47:31 AM UTC, comment #14: 

Sounds good, then.  The output of comment #10 looks like it addresses the problem.  Thanks!

Dave <barx>
Group Member
Thu 25 May 2023 11:34:02 AM UTC, comment #13: 


comment #12:

> comment #11:
> > +       * src/roff/troff/env.cpp (is_family_valid): New function checks
> > +       for all text styles (R, I, B, BI) and returns true only if the
> > +       given family supports them all.
> > +       (family_change): Call `is_family_valid()` on given argument.
> > +       If invalid, throw diagnostic and ignore `fam` request.
>
> If comment #9 is correct, wouldn't this disallow a CJK family?  Or do those fonts not use the family concept?


They don't, yet.  If they're going to, we need to make the "styles" concept considerably more flexible, and probably have it driven by new directives in the device and/or font description files.  I've had a couple of different ideas along these lines, and I don't like either one.

So I reckon we can cross that bridge later.

G. Branden Robinson <gbranden>
Group administrator
Thu 25 May 2023 11:30:01 AM UTC, comment #12: 

comment #11:

> +       * src/roff/troff/env.cpp (is_family_valid): New function checks
> +       for all text styles (R, I, B, BI) and returns true only if the
> +       given family supports them all.
> +       (family_change): Call `is_family_valid()` on given argument.
> +       If invalid, throw diagnostic and ignore `fam` request.


If comment #9 is correct, wouldn't this disallow a CJK family?  Or do those fonts not use the family concept?

Dave <barx>
Group Member
Thu 25 May 2023 10:48:23 AM UTC, comment #11: 

Here's what I've pushed to my working copy.


diff --git a/ChangeLog b/ChangeLog
index 79163d005..b91609090 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2023-05-25  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       [troff]: Validate a font family before trying to use it.
+
+       * src/roff/troff/env.cpp (is_family_valid): New function checks
+       for all text styles (R, I, B, BI) and returns true only if the
+       given family supports them all.
+       (family_change): Call `is_family_valid()` on given argument.
+       If invalid, throw diagnostic and ignore `fam` request.
+       * src/roff/troff/env.h (is_family_valid): Declare; make visible.
+       * src/roff/troff/input.cpp (main): Call `is_family_valid()` on
+       `-f` option argument.  Its invalidity is a fatal error.
+
+       Fixes <https://savannah.gnu.org/bugs/?64115>.  Thanks to Dave
+       Kemper for the report.
+
 2023-05-25  G. Branden Robinson <g.branden.robinson@gmail.com>

        * src/roff/troff/env.h (read_hyphen_file): Drop relic prototype
diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 4c64273ef..fc14cb715 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -1256,9 +1256,32 @@ void font_change()
   skip_line();
 }

+bool is_family_valid(const char *fam)
+{
+  // std::vector<const char *> styles{"R", "I", "B", "BI"}; // C++11
+  std::vector<const char *> styles;
+  styles.reserve(4);
+  styles.push_back("R");
+  styles.push_back("I");
+  styles.push_back("B");
+  styles.push_back("BI");
+  std::vector<const char *>::iterator style;
+  // for (auto style : styles) // C++11
+  for (style = styles.begin(); style != styles.end(); style++)
+    if (!check_font(fam, *style))
+      return false;
+  return true;
+}
+
 void family_change()
 {
   symbol s = get_name();
+  if (s != 0 /* nullptr */)
+    if (!is_family_valid(s.contents())) {
+      error("'%1' is not a valid font family", s.contents());
+      skip_line();
+      return;
+    }
   curenv->set_family(s);
   skip_line();
 }
diff --git a/src/roff/troff/env.h b/src/roff/troff/env.h
index d98941914..8382d11ea 100644
--- a/src/roff/troff/env.h
+++ b/src/roff/troff/env.h
@@ -408,6 +408,7 @@ extern void pop_env();
 extern void push_env(int);

 void init_environments();
+bool is_family_valid(const char *);

 extern double spread_limit;

diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index cbb8b1ca3..56c20f3eb 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -8333,6 +8333,9 @@ int main(int argc, char **argv)
        warning(WARN_FONT, "cannot mount font '%1' directed by 'DESC'"
                " file for device '%2'", font::font_name_table[i],
                device);
+  if (fflag && !(is_family_valid(default_family.contents())))
+      fatal("'%1' is not a valid font family",
+           default_family.contents());
   curdiv = topdiv = new top_level_diversion;
   if (nflag)
     topdiv->set_next_page_number(next_page_number);


G. Branden Robinson <gbranden>
Group administrator
Thu 25 May 2023 09:46:01 AM UTC, comment #10: 

Hi Dave,

How does this look?  (Apart from a little confusing, since this is mixed input and output.)


$ ./build/test-groff -b -ww
.tm .fam=\n[.fam]
.fam=T
.fam H
.tm .fam=\n[.fam]
.fam=H
.fam
.tm .fam=\n[.fam]
.fam=T
.fam ZD
troff: backtrace: file '<standard input>':6
troff:<standard input>:6: error: 'ZD' is not a family; missing style 'I'
.fam BOGUS
troff: backtrace: file '<standard input>':7
troff:<standard input>:7: error: 'BOGUS' is not a family; missing style 'R'
.tm .fam=\n[.fam]
.fam=T
.fam
.tm .fam=\n[.fam]
.fam=H


G. Branden Robinson <gbranden>
Group administrator
Sun 07 May 2023 05:05:31 PM UTC, comment #9: 

I don't think CJK fonts have I or BI variants (but they can have R and B). To view Japanese man pages something like this is used:-

groff -Tpdf -man -dHF=MB -kt -mja -fM -dpaper=a4 -P-pa4 groff.7  > groff.7.pdf

So it probably would be better to check if the R font of given -f fam contains latin-1 charset (don't need to check all - if 'a' and 'A' are in their latin-1 positions it would be sufficiently indicative).

NB gropdf support for multi thousand glyph fonts is in development, grops already supports.

Deri James <deri>
Group Member
Sun 07 May 2023 04:25:31 PM UTC, comment #8: 


comment #7:

>   Thus "ZD" is accepted as a family name,
> as the letter "R" is interpreted as meaning a roman font.


[and on the 'ps' device, a "ZDR" font description actually exists]

I noticed this myself, and is one reason I proposed checking all four styles when the user (attempts to) select a family.

G. Branden Robinson <gbranden>
Group administrator
Sun 07 May 2023 02:56:24 PM UTC, comment #7: 

  My "solution" has a side effect,
so it can't be integrated into the software.

  If an input file is used with some characters,
then it turns out,
that the font "ZDR" is used.

  Thus "ZD" is accepted as a family name,
as the letter "R" is interpreted as meaning a roman font.

Bjarni Ingi Gislason <bjarniig>
Sun 07 May 2023 02:35:41 AM UTC, comment #6: 


> Is it an error to specify a family where some font styles are missing if the document never asks to use those styles?


I'd characterize that as a warning.  I'd like to see the formatter check for the existence of all four styles when a family is requested.  (Or at startup if `-f` is specified.)
 

> Is it an error to specify a family where all font styles are missing if the document never actually uses that family? (e.g., specifying "-fZD" on the command line to process a document whose first line is ".fam T")


I'd make this a warning too.

And, more realistically, in the case where, on the "dvi" device, someone specified -fC, thinking they could get something equivalent to the "Courier" family.  But the Computer Modern fonts aren't quite up to that.

G. Branden Robinson <gbranden>
Group administrator
Sun 07 May 2023 01:57:25 AM UTC, comment #5: 

comment #3:

> Possibly a search for the requisite styles could be undertaken
> upon use of the `-f` flag or `fam` request, errors thrown for
> any that are missing, and the option or request ignored.


Hmm, that raises a couple of interesting questions.

Is it an error to specify a family where some font styles are missing if the document never asks to use those styles?

Is it an error to specify a family where all font styles are missing if the document never actually uses that family? (e.g., specifying "-fZD" on the command line to process a document whose first line is ".fam T")

Dave <barx>
Group Member
Sun 07 May 2023 01:38:45 AM UTC, comment #4: 

comment #3:

> However, Zapf Dingbats is a specific face, not a family.  And
> not a member of a family.


Whoops!  You're right.

> Remember this old chestnut?
>


>     troff: EXPERIMENTS/font-errors.groff:23: warning: can't find font 'BOGUSR'


>
> This is pretty close to the actual problem.  If "ZD" is the
> family, then the formatter expects to find fonts named "ZDR",
> "ZDI", "ZDB", and "ZDBI".


This could be what's going on, but the "character not defined" warnings suggest that groff is successfully (though perhaps wrongly) setting the font to something that lacks basic alphabetic characters (as ZD does), then running into the string-comparison issue Deri noted (comment #1).  If groff were seeking a phantom "ZDR" font, the font-setting should fail altogether, leaving the font as TR, which I wouldn't expect to produce those particular warnings.

> The numeric overflow is interesting.


#NoBoringBugs

Dave <barx>
Group Member
Sat 06 May 2023 05:16:42 PM UTC, comment #3: 

This is kind of a case of "don't do that, then".  ("Doctor, it hurts when I do this!")


    -f fam
        Set default font family.


However, Zapf Dingbats is a specific face, not a family.  And not a member of a family.

No matter what happens, you'll get diagnostics, but we probably could improve the diagnostics you get.

Remember this old chestnut?


    troff: EXPERIMENTS/font-errors.groff:23: warning: can't find font 'BOGUSR'


This is pretty close to the actual problem.  If "ZD" is the family, then the formatter expects to find fonts named "ZDR", "ZDI", "ZDB", and "ZDBI".

Possibly a search for the requisite styles could be undertaken upon use of the `-f` flag or `fam` request, errors thrown for any that are missing, and the option or request ignored.

The numeric overflow is interesting.

G. Branden Robinson <gbranden>
Group administrator
Sat 06 May 2023 04:57:10 PM UTC, comment #2: 

  I found a solution, it did though need thinking to find it.

test-groff -ww -b -z -fZD /dev/null

  runs then with no output and returns with zero as exit value.

  I will leave this bug as is, to give you (plural) a chance to find a
solution on your own.

Bjarni Ingi Gislason <bjarniig>
Sat 06 May 2023 12:20:15 PM UTC, comment #1: 

I suspect this is related to bug #64144 (errors when setting -f to a font which does not contain a full latin-1 character set). It occurs when using .if ''' in a macro so a construct such as .if '\*[.T]'html' will fail when troff generates the output of both to compare.

Branden devised an ingenious solution in bug #64144, it is a bit odd because -T pdf produces a different error:-

[derij@pip build (new-gropdf)]$ echo ' ' | ./test-groff -Tps -fEURO -a
troff: fatal error: invalid default font family 'EURO'
[derij@pip build (new-gropdf)]$ echo ' ' | ./test-groff -Tpdf -fEURO -a
troff: fatal error: invalid default font family 'EURO'
[derij@pip build (new-gropdf)]$ echo ' ' | ./test-groff -Tpdf -fZD -a
troff: fatal error: invalid default font family 'ZD'
[derij@pip build (new-gropdf)]$ echo ' ' | ./test-groff -Tps -fZD -a
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:14: warning: character 'p' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:14: warning: character 's' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:14: warning: character 'a' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:14: warning: character 'c' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:14: warning: character 'i' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:16: warning: character 'l' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:16: warning: character 't' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:16: warning: character 'n' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:17: warning: character 'u' not defined
troff:/home/derij/groff-git/groff/build/../tmac/fallbacks.tmac:17: warning: character 'f' not defined
troff:/home/derij/groff-git/groff/build/../tmac/troffrc-end:7: warning: character 'h' not defined
troff:/home/derij/groff-git/groff/build/../tmac/troffrc-end:7: warning: character 'm' not defined
<beginning of page>

troff:/home/derij/groff-git/groff/build/../tmac/html-end.tmac:20: error: numeric overflow


Deri James <deri>
Group Member
Fri 05 May 2023 11:41:19 PM UTC, original submission:  

This bug was present in 1.22.4:


$ echo ' ' | groff -fZD -a
troff: /usr/share/groff/1.22.4/tmac/ps.tmac:686: warning: can't find character 'a'
troff: /usr/share/groff/1.22.4/tmac/troffrc-end:7: warning: can't find character 'p'
troff: /usr/share/groff/1.22.4/tmac/troffrc-end:7: warning: can't find character 's'
troff: /usr/share/groff/1.22.4/tmac/troffrc-end:7: warning: can't find character 'h'
troff: /usr/share/groff/1.22.4/tmac/troffrc-end:7: warning: can't find character 't'
troff: /usr/share/groff/1.22.4/tmac/troffrc-end:7: warning: can't find character 'm'
troff: /usr/share/groff/1.22.4/tmac/troffrc-end:7: warning: can't find character 'l'
<beginning of page>

troff: /usr/share/groff/1.22.4/tmac/html-end.tmac:20: numeric overflow


But the number of warnings increased in 1.23.  (Pathnames below trimmed.)

$ groff --version | head -1
GNU groff version 1.23.0.rc4.19-96b92-dirty
$ echo ' ' | groff -fZD -a
troff:.../tmac/fallbacks.tmac:14: warning: character 'p' not defined
troff:.../tmac/fallbacks.tmac:14: warning: character 's' not defined
troff:.../tmac/fallbacks.tmac:14: warning: character 'a' not defined
troff:.../tmac/fallbacks.tmac:14: warning: character 'c' not defined
troff:.../tmac/fallbacks.tmac:14: warning: character 'i' not defined
troff:.../tmac/fallbacks.tmac:16: warning: character 'l' not defined
troff:.../tmac/fallbacks.tmac:16: warning: character 't' not defined
troff:.../tmac/fallbacks.tmac:16: warning: character 'n' not defined
troff:.../tmac/fallbacks.tmac:17: warning: character 'u' not defined
troff:.../tmac/fallbacks.tmac:17: warning: character 'f' not defined
troff:.../tmac/troffrc-end:7: warning: character 'h' not defined
troff:.../tmac/troffrc-end:7: warning: character 'm' not defined
<beginning of page>

troff:.../tmac/html-end.tmac:20: error: numeric overflow


Dave <barx>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Posted a comment)
  • -email is unavailable- added by bjarniig (Posted a comment)
  • -email is unavailable- added by deri (Posted a comment)
  • -email is unavailable- added by barx (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-07-10 gbranden StatusReady for Merge Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.24.0
        Summary[PATCH] specifying -fZD on command line generates warnings specifying -fZD on command line generates warnings
    2023-06-01 gbranden StatusIn Progress Ready for Merge
    2023-05-25 gbranden Summaryspecifying -fZD on command line generates warnings [PATCH] specifying -fZD on command line generates warnings
    2023-05-25 gbranden StatusNone In Progress
        Assigned toNone gbranden

    Back to the top

    Powered by Savane 3.12