[groff] 61/115: [mm]: Fix Savannah #64205.

[G. Branden Robinson]

gbranden pushed a commit to branch branden-2022-06-01
in repository groff.

commit 39ce09b2c58faafc1f4119a6152d36cba46b33c7
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Thu May 18 03:38:54 2023 -0500

    [mm]: Fix Savannah #64205.
    
    [mm]: Improve input validation of `INITI`, `IND` macros.
    
    * contrib/mm/m.tmac (INITI): Validate the index type (first argument)
      here instead of only later when `IND` is called.  Actually verify that
      the second argument is present and usable.
    
      (IND): Verify that we're given arguments.
    
    Fixes <https://savannah.gnu.org/bugs/?64205>.
    
    Also make some design observations in comments.
    
    Tested as follows using groff 1.22.4 and Git HEAD (on my branch).
    
    $ cat EXPERIMENTS/test-index.sh
    \#!/bin/sh
    
    groff=./build/test-groff
    groff=groff
    
    printf '.INITI\n' | $groff -mm
    printf '.INITI "" ""\n' | $groff -mm
    printf '.INITI A ""\n' | $groff -mm
    printf '.INITI N ""\n' | $groff -mm
    printf '.INITI N myindex\n.INITI N myotherindex\n' | $groff -mm
    printf '.IND foo\n' | $groff -mm
    printf '.INITI N myindex\n.IND\n' | $groff -mm
    printf '.INITI N myindex\n.IND ""\n' | $groff -mm
    printf '.INITI N myindex\n.IND foo\n' | $groff -mm
    $ sh EXPERIMENTS/test-index.sh
    ******************
    ERROR:(<standard input>) input line 1:INITI:type missing
    ******************
    "Input aborted, syntax error"
    ******************
    ERROR:(<standard input>) input line 2:INITI:file already set
    ******************
    "Input aborted, syntax error"
    ******************
    ERROR:(<standard input>) input line 1:IND: No active INITI
    ******************
    "Input aborted, syntax error"
    
    Removing the second assignment to the "groff" shell variable...
    
    $ sh EXPERIMENTS/test-index.sh
    m.tmac:<standard input>:1: error: INITI: expected 2 or 3 arguments, got 0
    m.tmac:<standard input>:1: error: INITI: invalid index type ''
    m.tmac:<standard input>:1: error: INITI: invalid index type 'A'
    m.tmac:<standard input>:1: error: INITI: index file name not specified
    m.tmac:<standard input>:2: error: INITI: index file name already set
    m.tmac:<standard input>:1: error: IND: no active index; call INITI first
    m.tmac:<standard input>:2: error: IND: expected at least 1 argument, got 0
    m.tmac:<standard input>:2: error: IND: cannot record empty index entry
---
 contrib/mm/ChangeLog | 11 +++++++++++
 contrib/mm/m.tmac    | 35 +++++++++++++++++++++++------------
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/contrib/mm/ChangeLog b/contrib/mm/ChangeLog
index 323813e14..15f97d737 100644
--- a/contrib/mm/ChangeLog
+++ b/contrib/mm/ChangeLog
@@ -1,3 +1,14 @@
+2023-05-18  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       Improve input validation of `INITI`, `IND` macros.
+
+       * m.tmac (INITI): Validate the index type (first argument) here
+       instead of only later when `IND` is called.  Actually verify
+       that the second argument is present and usable.
+       (IND): Verify that we're given arguments.
+
+       Fixes <https://savannah.gnu.org/bugs/?64205>.
+
 2023-05-18  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        * m.tmac (Indcmd): Drop relic string.  Nothing interpolates it;
diff --git a/contrib/mm/m.tmac b/contrib/mm/m.tmac
index 16759bf88..851974f69 100644
--- a/contrib/mm/m.tmac
+++ b/contrib/mm/m.tmac
@@ -3330,30 +3330,41 @@ exceeds depth of nested lists (\\n[li*lvl])
 ..
 .\"########################### module ind ############################
 .\"--------------------
-.\" Another type of index system
+.\" initialize index
 .\" INITI type filename [macro]
+.\" XXX: Since we can build only one index at a time[1], it shouldn't be
+.\" necessary to have a mandatory filename argument[2].  Consider
+.\" letting the mmroff Perl script generate a temporary filename for us.
+.\"
+.\" [1] Why?  Indexes go through Perl anyway, and it can keep track of
+.\" an arbitrary number of lists to sort independently.
+.\"
+.\" [2] Just "name" would be a better term for it since we
+.\" unconditionally suffix it with ".ind".
 .de INITI
 .if \\n[.$]<2 .@error \\$0: expected 2 or 3 arguments, got \\n[.$]
-.\" ignore if INITI has already been used
+.ds ind*ref \" empty
+.ds ind*type \\$1
+.if '\\*[ind*type]'N' .ds ind*ref \\n[%]
+.if '\\*[ind*type]'H' .ds ind*ref \\*[hd*mark]
+.if '\\*[ind*type]'B' .ds ind*ref \\*[hd*mark]\t\\n[%]
+.if '\\*[ind*ref]'' .@error \\$0: invalid index type '\\$1'
+.if \\n[D]>1 .tm INITI: type=\\*[ind*type]
+.if '\\$2'' .@error \\$0: index file name not specified
 .if d ind*file .@error \\$0: index file name already set
 .ds ind*file \\$2.ind
 .if \\n[D]>1 .tm INITI: source \\*[ind*file]
-.if !d ind*file .@error \\$0: index file name not specified
-.ds ind*type \\$1
 .if \\n[Ref] \{\
 .      if \\n[.$]>2 .tm .\\\\" Imacro: \\$3
 .\}
 ..
 .\"---------------
+.\" collect index entry
+.\" IND term ...
 .de IND
-.if !d ind*file .@error \\$0: no active index; call INITI"
-.if \\n[D]>1 .tm IND: type=\\*[ind*type]
-.ds ind*ref \" empty
-.if '\\*[ind*type]'N' .ds ind*ref \\n[%]
-.if '\\*[ind*type]'H' .ds ind*ref \\*[hd*mark]
-.if '\\*[ind*type]'B' .ds ind*ref \\*[hd*mark]\t\\n[%]
-.if '\\*[ind*ref]'' .@abort invalid index type '\\*[ind*type]'
-.\"
+.if !d ind*file .@error \\$0: no active index; call INITI first
+.if \\n[.$]<1 .@error \\$0: expected at least 1 argument, got \\n[.$]
+.if '\\$1'' .@error \\$0: cannot record empty index entry
 .ds ind*line \\$1
 .while \\n[.$]>0 \{\
 .      shift