gbranden pushed a commit to branch master
in repository groff.

commit f0668fcbb70f7a126a07c761706610b3c42bda51
Author: G. Branden Robinson <g.branden.robin...@gmail.com>
AuthorDate: Sun Feb 23 17:48:16 2025 -0600

    [troff]: Improve `file_iterator`'s memory mgmt.
    
    * src/roff/troff/input.cpp (class file_iterator): Make class manage its
      own memory for storing file names, instead of reusing externally
      managed pointers (which sometimes refer to statically allocated
      storage, as with "-" representing the standard input stream).
      De-`const` `filename` member variable.
    
      (file_iterator::file_iterator): Constructor now uses strdup(3) to copy
      the supplied file name instead of just a pointer to it via the
      initializer list.  (In C++, tastes differ whether a constructor should
      call [non-virtual] member functions.  In this case, I don't.)
    
      (file_iterator::next_file): Call member function `set_location()`
      instead of manipulating member variables, to ensure reliable memory
      management.
    
      (file_iterator::set_location): Use strdup(3) to allocate storage for
      the file name and use the returned pointer.  If the file name pointer
      is not null, free(3) it first.
---
 ChangeLog                | 19 +++++++++++++++++++
 src/roff/troff/input.cpp | 21 +++++++++++++--------
 2 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 758a72866..a4d0b4926 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2025-02-23  G. Branden Robinson <g.branden.robin...@gmail.com>
+
+       * src/roff/troff/input.cpp (class file_iterator): Make class
+       manage its own memory for storing file names, instead of reusing
+       externally managed pointers (which sometimes refer to statically
+       allocated storage, as with "-" representing the standard input
+       stream).  De-`const` `filename` member variable.
+       (file_iterator::file_iterator): Constructor now uses strdup(3)
+       to copy the supplied file name instead of just a pointer to it
+       via the initializer list.  (In C++, tastes differ whether a
+       constructor should call [non-virtual] member functions.  In this
+       case, I don't.)
+       (file_iterator::next_file): Call member function
+       `set_location()` instead of manipulating member variables, to
+       ensure reliable memory management.
+       (file_iterator::set_location): Use strdup(3) to allocate storage
+       for the file name and use the returned pointer.  If the file
+       name pointer is not null, free(3) it first.
+
 2025-02-22  G. Branden Robinson <g.branden.robin...@gmail.com>
 
        * src/libs/libgroff/font.cpp: Include "string.h" standard header
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index f67090bef..2b1f0faff 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -397,7 +397,7 @@ public:
 class file_iterator : public input_iterator {
   FILE *fp;
   int lineno;
-  const char *filename;
+  char *filename;
   bool was_popened;
   bool seen_newline;
   bool seen_escape;
@@ -418,9 +418,10 @@ public:
 };
 
 file_iterator::file_iterator(FILE *f, const char *fn, bool popened)
-: fp(f), lineno(1), filename(fn), was_popened(popened),
+: fp(f), lineno(1), was_popened(popened),
   seen_newline(false), seen_escape(false)
 {
+  filename = strdup(const_cast<char *>(fn));
   if ((font::use_charnames_in_special) && (fn != 0 /* nullptr */)) {
     if (!the_output)
       init_output();
@@ -446,9 +447,8 @@ void file_iterator::close()
 bool file_iterator::next_file(FILE *f, const char *s)
 {
   close();
-  filename = s;
   fp = f;
-  lineno = 1;
+  set_location(s, 1);
   seen_newline = false;
   seen_escape = false;
   was_popened = false;
@@ -528,8 +528,13 @@ void file_iterator::backtrace()
 
 bool file_iterator::set_location(const char *f, int ln)
 {
-  if (f != 0 /* nullptr */)
-    filename = f;
+  if (f != 0 /* nullptr */) {
+    if (filename != 0 /* nullptr */) {
+      free(filename);
+      filename = 0 /* nullptr */;
+    }
+    filename = strdup(const_cast<char *>(f));
+  }
   lineno = ln;
   return true;
 }
@@ -8556,7 +8561,7 @@ void abort_request()
 // The caller must subsequently call `tok.next()` to advance the input
 // stream pointer.
 //
-// The caller has responsibility for freeing the returned array.
+// The caller has responsibility for `delete`ing the returned array.
 char *read_string()
 {
   int len = 256;
@@ -9850,7 +9855,7 @@ static void do_error(error_type type,
     input_stack::backtrace();
   if (!get_file_line(&filename, &lineno))
     filename = 0 /* nullptr */;
-  if (filename) {
+  if (filename != 0 /* nullptr */) {
     if (program_name)
       errprint("%1:", program_name);
     errprint("%1:%2: ", filename, lineno);

_______________________________________________
groff-commit mailing list
groff-commit@gnu.org
https://lists.gnu.org/mailman/listinfo/groff-commit

Reply via email to