groff-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[groff] 09/12: [tbl]: Refactor for less inefficiency.


From: G. Branden Robinson
Subject: [groff] 09/12: [tbl]: Refactor for less inefficiency.
Date: Mon, 14 Oct 2024 06:20:15 -0400 (EDT)

gbranden pushed a commit to branch master
in repository groff.

commit 1a2fa76cad761f57aa07cfaa488f7d5be19b686a
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Sun Oct 13 13:41:51 2024 -0500

    [tbl]: Refactor for less inefficiency.
    
    * src/preproc/tbl/table.cpp (table::add_entry): Refactor.  Since we use
      the C string `extract()`ion of the groff `string` table entry
      repeatedly, move logic that stores its pointer to an automatic
      variable much earlier, and reference that in diagnostic messages
      instead of repeatedly calling a member function.
    
    A thing to know about groff's `string::extract()` is that it allocates
    memory.[1]  One might wonder, as I did, when that memory is ever freed.
    The answer is: when tbl(1) exits.  That's because tbl needs these string
    pointers after `table::add_entry()` returns, or there won't be much in
    your table.  Ah, to program like it's 1989 again.
    
    A more fastidious approach would copy the pointers to each of these
    allocations into an STL `vector` (possibly a private member of class
    `table`), and then, when a table region is finished (`.T&` or `.TE` is
    encountered), walk the vector and free the storage.
    
    [1] 
https://git.savannah.gnu.org/cgit/groff.git/tree/src/libs/libgroff/string.cpp?h=1.23.0#n285
---
 ChangeLog                 |  9 +++++++++
 src/preproc/tbl/table.cpp | 13 +++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 221c3fa1d..5c4c9a144 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2024-10-13  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       * src/preproc/tbl/table.cpp (table::add_entry): Refactor.  Since
+       we use the C string `extract()`ion of the groff `string` table
+       entry repeatedly, move logic that stores its pointer to an
+       automatic variable much earlier, and reference that in
+       diagnostic messages instead of repeatedly calling a member
+       function.
+
 2024-10-12  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        * src/preproc/preconv/preconv.cpp (do_file): Fix incorrect
diff --git a/src/preproc/tbl/table.cpp b/src/preproc/tbl/table.cpp
index 3ffbe3471..612c93666 100644
--- a/src/preproc/tbl/table.cpp
+++ b/src/preproc/tbl/table.cpp
@@ -1526,6 +1526,7 @@ void table::add_entry(int r, int c, const string &str,
   allocate(r);
   table_entry *e = 0 /* nullptr */;
   int len = str.length();
+  char *s = str.extract();
   // Diagnose escape sequences that can wreak havoc in generated output.
   if (len > 1) {
     // A comment on a control line or in a text block is okay.
@@ -1536,8 +1537,7 @@ void table::add_entry(int r, int c, const string &str,
          && ((-1 == controlpos) // (no control character in line OR
              || (0 == controlpos))) // control character at line start)
        warning_with_file_and_line(fn, ln, "comment escape sequence"
-                                  " '\\\"' in entry \"%1\"",
-                                  str.extract());
+                                  " '\\\"' in entry \"%1\"", s);
     }
     int gcommentpos = str.find("\\#");
     // If both types of comment are present, the first is what matters.
@@ -1549,8 +1549,7 @@ void table::add_entry(int r, int c, const string &str,
          && ((-1 == controlpos) // (no control character in line OR
              || (0 == controlpos))) // control character at line start)
        warning_with_file_and_line(fn, ln, "comment escape sequence"
-                                  " '\\#' in entry \"%1\"",
-                                  str.extract());
+                                  " '\\#' in entry \"%1\"", s);
     }
     // A \! escape sequence after a comment has started is okay.
     int exclpos = str.find("\\!");
@@ -1560,7 +1559,7 @@ void table::add_entry(int r, int c, const string &str,
       if (-1 == str.search('\n')) // not a text block
        warning_with_file_and_line(fn, ln, "transparent throughput"
                                   " escape sequence '\\!' in entry"
-                                  " \"%1\"", str.extract());
+                                  " \"%1\"", s);
       else
        warning_with_file_and_line(fn, ln, "transparent throughput"
                                   " escape sequence '\\!' in text"
@@ -1570,14 +1569,12 @@ void table::add_entry(int r, int c, const string &str,
     if (str.find("\\z") == (len - 2)) { // max valid index is (len - 1)
       if (-1 == str.search('\n')) // not a text block
        error_with_file_and_line(fn, ln, "zero-motion escape sequence"
-                                " '\\z' at end of entry \"%1\"",
-                                str.extract());
+                                " '\\z' at end of entry \"%1\"", s);
       else
        error_with_file_and_line(fn, ln, "zero-motion escape sequence"
                                 " '\\z' at end of text block entry");
     }
   }
-  char *s = str.extract();
   if (str.search('\n') != -1) { // if it's a text block
     bool was_changed = false;
     int repeatpos = str.find("\\R");



reply via email to

[Prev in Thread] Current Thread [Next in Thread]