[groff] 99/126: [troff]: Fix Savannah #64104 (2/2).

[G. Branden Robinson]

gbranden pushed a commit to branch branden-2023-07-05
in repository groff.

commit 7c1f63901bdf652b7940617b206f14fd93393945
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Sun May 7 12:33:20 2023 -0500

    [troff]: Fix Savannah #64104 (2/2).
    
    * src/roff/troff/env.h (class environment): Slightly refactor.  Rename
      fields `control_char` and `no_break_control_char` to
      `control_character` and `no_break_control_character`, respectively,
      and make them private.  Add public getters and setters for them.
    
      (get_control_character): New public member function returns `unsigned
      char`.
    
      (set_control_character): New public member function takes an `unsigned
      char` and returns a `bool`.
    
      (get_no_break_control_character):
      (set_no_break_control_character): Similar.
    
    * src/roff/troff/env.cpp (environment::environment): Update constructors
      to use new field names and initialize them in their new order of
      declaration.
    
      (environment::copy): Similar.  (Not to be confused with a copy
      constructor, this member function implements the operation of a valid
      `evc` GNU troff request.)
    
      (environment::get_control_character):
      (environment::set_control_character):
      (environment::get_no_break_control_character):
      (environment::set_no_break_control_character): Implement.
    
    * src/roff/troff/env.cpp: Drop handling of `cc` and `c2` requests.
    
      (control_char, no_break_control_char): Drop implementations.
    
      (init_env_requests): Drop `init_request` calls for them.
    
    * src/roff/troff/input.cpp: Resurrect handling of `cc` and `c2` requests
      here, with more error handling.
    
      (assign_escape_character, process_input_stack): Use the new getters
      defined above in environment object since the relevant fields are now
      private.
    
      (assign_control_character): New function assumes the former
      responsibilities of env.cpp:control_char and rejects the request if
      the desired character is already the escape or no-break control
      character.
    
      (assign_no_break_control_character): New function assumes the former
      responsibilities of env.cpp:no_break_control_char and rejects the
      request if the desired character is already the escape or control
      character.
    
      (process_input_stack): Reorder comparisons to avoid inadvertent lvalue
      assignment.  Add parentheses to assignment with complex RHS.
    
      (init_input_requests): Attach `cc` and `c2` requests to new functions
      above.
    
    * doc/groff.texi (Control Characters): Document restrictions.
    
    Fixes <https://savannah.gnu.org/bugs/?64104>.
    
    Tested with:
    
    $ cat cc.roff
    .cc \\
    .cc '
    .cc \-
    $ ./build/test-groff cc.roff
    troff:cc.roff:1: error: ignoring control character change request; the 
escape character is already '\'
    troff:cc.roff:2: error: ignoring control character change request; the 
no-break control character is already "'"
    troff:cc.roff:3: error: cannot select invalid control character; using '.'
    $ cat c2.roff
    .c2 \\
    .c2 .
    .c2 \-
    $ ./build/test-groff c2.roff
    troff:c2.roff:1: error: ignoring no-break control character change request; 
the escape character is already '\'
    troff:c2.roff:2: error: ignoring no-break control character change request; 
the (breaking) control character is already '.'
    troff:c2.roff:3: error: cannot select invalid no-break control character; 
using "'"
---
 ChangeLog                | 47 +++++++++++++++++++++++++
 doc/groff.texi           |  8 +++--
 src/roff/troff/env.cpp   | 66 +++++++++++++++++------------------
 src/roff/troff/env.h     | 12 ++++---
 src/roff/troff/input.cpp | 90 ++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 178 insertions(+), 45 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 023be3714..99f4b7cd2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -9,6 +9,53 @@
        * doc/groff.texi (Using Escape Sequences): Document
        restrictions.
 
+       * src/roff/troff/env.h (class environment): Slightly refactor.
+       Rename fields `control_char` and `no_break_control_char` to
+       `control_character` and `no_break_control_character`,
+       respectively, and make them private.  Add public getters and
+       setters for them.
+       (get_control_character): New public member function returns
+       `unsigned char`.
+       (set_control_character): New public member function takes an
+       `unsigned char` and returns a `bool`.
+       (get_no_break_control_character):
+       (set_no_break_control_character): Similar.
+       * src/roff/troff/env.cpp (environment::environment): Update
+       constructors to use new field names and initialize them in their
+       new order of declaration.
+       (environment::copy): Similar.  (Not to be confused with a copy
+       constructor, this member function implements the operation of a
+       valid `evc` GNU troff request.)
+       (environment::get_control_character):
+       (environment::set_control_character):
+       (environment::get_no_break_control_character):
+       (environment::set_no_break_control_character): Implement.
+
+       * src/roff/troff/env.cpp: Drop handling of `cc` and `c2`
+       requests.
+       (control_char, no_break_control_char): Drop implementations.
+       (init_env_requests): Drop `init_request` calls for them.
+
+       * src/roff/troff/input.cpp: Resurrect handling of `cc` and `c2`
+       requests here, with more error handling.
+       (assign_escape_character, process_input_stack): Use the new
+       getters defined above in environment object since the relevant
+       fields are now private.
+       (assign_control_character): New function assumes the former
+       responsibilities of env.cpp:control_char and rejects the request
+       if the desired character is already the escape or no-break
+       control character.
+       (assign_no_break_control_character): New function assumes the
+       former responsibilities of env.cpp:no_break_control_char and
+       rejects the request if the desired character is already the
+       escape or control character.
+       (process_input_stack): Reorder comparisons to avoid inadvertent
+       lvalue assignment.  Add parentheses to assignment with complex
+       RHS.
+       (init_input_requests): Attach `cc` and `c2` requests to new
+       functions above.
+       * doc/groff.texi (Control Characters): Document restrictions.
+
 2023-05-07  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        [troff]: Don't quote a character with itself in diagnostics.
diff --git a/doc/groff.texi b/doc/groff.texi
index 12c732fe0..dba93044b 100644
--- a/doc/groff.texi
+++ b/doc/groff.texi
@@ -6636,14 +6636,18 @@ with the @code{cc} and @code{c2} requests, respectively.
 @Defreq {cc, [@Var{o}]}
 Recognize the ordinary character@tie{}@var{o} as the control character.
 If@tie{}@var{o} is absent or invalid, the default control character
-@samp{.} is selected.  The identity of the control character is
+@samp{.} is selected.  If @var{o} (or @samp{.} if @var{o} is invalid) is
+already the escape or no-break control character, an error is diagnosed
+and the request ignored.  The identity of the control character is
 associated with the environment (@pxref{Environments}).
 @endDefreq
 
 @Defreq {c2, [@Var{o}]}
 Recognize the ordinary character@tie{}@var{o} as the no-break control
 character.  If@tie{}@var{o} is absent or invalid, the default no-break
-control character @samp{'} is selected.  The identity of the no-break
+control character @samp{'} is selected.  If @var{o} (or @samp{'} if
+@var{o} is invalid) is already the escape or control character, an error
+is diagnosed and the request ignored.  The identity of the no-break
 control character is associated with the environment
 (@pxref{Environments}).
 @endDefreq
diff --git a/src/roff/troff/env.cpp b/src/roff/troff/env.cpp
index 9f00284c6..4c64273ef 100644
--- a/src/roff/troff/env.cpp
+++ b/src/roff/troff/env.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2023 Free Software Foundation, Inc.
      Written by James Clark (jjc@jclark.com)
 
 This file is part of groff.
@@ -723,14 +723,14 @@ environment::environment(symbol nm)
   prev_glyph_color(&default_color),
   fill_color(&default_color),
   prev_fill_color(&default_color),
+  control_character('.'),
+  no_break_control_character('\''),
   seen_space(0),
   seen_eol(0),
   suppress_next_eol(0),
   seen_break(0),
   tabs(units_per_inch/2, TAB_LEFT),
   name(nm),
-  control_char('.'),
-  no_break_control_char('\''),
   hyphen_indicator_char(0)
 {
   prev_family = family = lookup_family(default_family);
@@ -816,14 +816,14 @@ environment::environment(const environment *e)
   prev_glyph_color(e->prev_glyph_color),
   fill_color(e->fill_color),
   prev_fill_color(e->prev_fill_color),
+  control_character(e->control_character),
+  no_break_control_character(e->no_break_control_character),
   seen_space(e->seen_space),
   seen_eol(e->seen_eol),
   suppress_next_eol(e->suppress_next_eol),
   seen_break(e->seen_break),
   tabs(e->tabs),
   name(e->name),               // so that, e.g., '.if "\n[.ev]"0"' works
-  control_char(e->control_char),
-  no_break_control_char(e->no_break_control_char),
   hyphen_indicator_char(e->hyphen_indicator_char)
 {
 }
@@ -866,8 +866,8 @@ void environment::copy(const environment *e)
   width_total = 0;
   space_total = 0;
   input_line_start = 0;
-  control_char = e->control_char;
-  no_break_control_char = e->no_break_control_char;
+  control_character = e->control_character;
+  no_break_control_character = e->no_break_control_character;
   hyphen_indicator_char = e->hyphen_indicator_char;
   spread_flag = 0;
   line = 0;
@@ -916,6 +916,32 @@ environment::~environment()
   delete_node_list(numbering_nodes);
 }
 
+unsigned char environment::get_control_character()
+{
+  return control_character;
+}
+
+bool environment::set_control_character(unsigned char c)
+{
+  if (c == no_break_control_character)
+    return false;
+  control_character = c;
+  return true;
+}
+
+unsigned char environment::get_no_break_control_character()
+{
+  return no_break_control_character;
+}
+
+bool environment::set_no_break_control_character(unsigned char c)
+{
+  if (c == control_character)
+    return false;
+  no_break_control_character = c;
+  return true;
+}
+
 hunits environment::get_input_line_position()
 {
   hunits n;
@@ -1534,30 +1560,6 @@ void underline()
   do_underline(0);
 }
 
-void control_char()
-{
-  curenv->control_char = '.';
-  if (has_arg()) {
-    if (tok.ch() == 0)
-      error("bad control character");
-    else
-      curenv->control_char = tok.ch();
-  }
-  skip_line();
-}
-
-void no_break_control_char()
-{
-  curenv->no_break_control_char = '\'';
-  if (has_arg()) {
-    if (tok.ch() == 0)
-      error("bad control character");
-    else
-      curenv->no_break_control_char = tok.ch();
-  }
-  skip_line();
-}
-
 void margin_character()
 {
   while (tok.is_space())
@@ -3423,8 +3425,6 @@ void init_env_requests()
   init_request("ad", adjust);
   init_request("br", break_request);
   init_request("brp", break_spread_request);
-  init_request("c2", no_break_control_char);
-  init_request("cc", control_char);
   init_request("ce", center);
   init_request("cu", continuous_underline);
   init_request("ev", environment_switch);
diff --git a/src/roff/troff/env.h b/src/roff/troff/env.h
index f6c1d211c..d196ab869 100644
--- a/src/roff/troff/env.h
+++ b/src/roff/troff/env.h
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2020 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2023 Free Software Foundation, Inc.
      Written by James Clark (jjc@jclark.com)
 
 This file is part of groff.
@@ -225,6 +225,8 @@ class environment {
   color *prev_glyph_color;
   color *fill_color;
   color *prev_fill_color;
+  unsigned char control_character;
+  unsigned char no_break_control_character;
 
   tab_type distance_to_next_tab(hunits *);
   tab_type distance_to_next_tab(hunits *distance, hunits *leftpos);
@@ -251,13 +253,15 @@ public:
   int seen_break;
   tab_stops tabs;
   const symbol name;
-  unsigned char control_char;
-  unsigned char no_break_control_char;
   charinfo *hyphen_indicator_char;
-  
+
   environment(symbol);
   environment(const environment *);    // for temporary environment
   ~environment();
+  unsigned char get_control_character();
+  bool set_control_character(unsigned char);
+  unsigned char get_no_break_control_character();
+  bool set_no_break_control_character(unsigned char);
   statem *construct_state(int only_eol);
   void print_env();
   void copy(const environment *);
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index f0538ec78..3079899dd 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2022 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2023 Free Software Foundation, Inc.
      Written by James Clark (jjc@jclark.com)
 
 This file is part of groff.
@@ -175,11 +175,11 @@ static void assign_escape_character()
   char already_cc[] = "the control character is already";
   char already_nbcc[] = "the no-break control character is already";
   char *already_message = 0 /* nullptr */;
-  if (ec == curenv->control_char) {
+  if (curenv->get_control_character() == ec) {
       already_message = already_cc;
       do_nothing = true;
   }
-  else if (ec == curenv->no_break_control_char) {
+  else if (curenv->get_no_break_control_character() == ec) {
       already_message = already_nbcc;
       do_nothing = true;
   }
@@ -216,6 +216,82 @@ void restore_escape_char()
   skip_line();
 }
 
+void assign_control_character()
+{
+  char cc = '\0';
+  bool is_invalid = false;
+  if (has_arg()) {
+    if (tok.ch() == 0)
+      is_invalid = true;
+    else
+      cc = tok.ch();
+  }
+  else
+    cc = '\\';
+  bool do_nothing = false;
+  char already_ec[] = "the escape character is already";
+  char already_nbcc[] = "the no-break control character is already";
+  char *already_message = 0 /* nullptr */;
+  if (cc == escape_char) {
+      already_message = already_ec;
+      do_nothing = true;
+  }
+  else if (curenv->get_no_break_control_character() == cc) {
+      already_message = already_nbcc;
+      do_nothing = true;
+  }
+  if (do_nothing)
+    error("ignoring control character change request; %1%2 %3",
+         is_invalid ? "cannot select invalid control character, and"
+         : "", already_message, input_char_description(cc));
+  else if (is_invalid) {
+    error("cannot select invalid control character; using '.'");
+    assert(curenv->set_control_character('.'));
+  }
+  else
+    assert(curenv->set_control_character(cc));
+  skip_line();
+}
+
+void assign_no_break_control_character()
+{
+  char nbcc = '\0';
+  bool is_invalid = false;
+  if (has_arg()) {
+    if (tok.ch() == 0)
+      is_invalid = true;
+    else
+      nbcc = tok.ch();
+  }
+  else
+    nbcc = '\\';
+  bool do_nothing = false;
+  char already_ec[] = "the escape character is already";
+  char already_cc[] = "the (breaking) control character is already";
+  char *already_message = 0 /* nullptr */;
+  if (nbcc == escape_char) {
+      already_message = already_ec;
+      do_nothing = true;
+  }
+  else if (curenv->get_control_character() == nbcc) {
+      already_message = already_cc;
+      do_nothing = true;
+  }
+  if (do_nothing)
+    error("ignoring no-break control character change request; %1%2 %3",
+         is_invalid ? "cannot select invalid no-break control"
+                      " character, and"
+         : "", already_message, input_char_description(nbcc));
+  else if (is_invalid) {
+    error("cannot select invalid no-break control character;"
+         " using \"\'\"");
+    assert(curenv->set_no_break_control_character('\''));
+  }
+  else
+    assert(curenv->set_no_break_control_character(nbcc));
+  skip_line();
+}
+
 struct arg_list;
 
 class input_iterator {
@@ -2867,9 +2943,9 @@ void process_input_stack()
       {
        unsigned char ch = tok.c;
        if (bol && !have_input
-           && (ch == curenv->control_char
-               || ch == curenv->no_break_control_char)) {
-         break_flag = ch == curenv->control_char;
+           && (curenv->get_control_character() == ch
+               || curenv->get_no_break_control_character() == ch)) {
+         break_flag = (curenv->get_control_character() == ch);
          // skip tabs as well as spaces here
          do {
            tok.next();
@@ -8388,6 +8464,8 @@ void init_input_requests()
   init_request("backtrace", backtrace_request);
   init_request("blm", blank_line_macro);
   init_request("break", while_break_request);
+  init_request("cc", assign_control_character);
+  init_request("c2", assign_no_break_control_character);
   init_request("cf", copy_file);
   init_request("cflags", char_flags);
   init_request("char", define_character);