gbranden pushed a commit to branch master
in repository groff.

commit ad74e0653c1092eaadc6b3606b2297bf4f7e910e
Author: G. Branden Robinson <[email protected]>
AuthorDate: Wed Jun 3 22:14:50 2026 -0500

    [grohtml]: Fix memory leak.
    
    * src/devices/grohtml/post-html.cpp: Preprocessor-include C++ "<stack>"
      header file.
    
      (class char_buffer): Add new private member variable `strings` of type
      `std::stack<char *>`, to keep track of the strdup(3)ped pointers that
      our `add_string()` member functions return.
    
      (char_buffer::char_buffer): Initialize `strings`.
    
      (char_buffer::~char_buffer): Pop `strings` stack, free(3)ing
      strdup(3)ped pointers until empty.
    
      (char_buffer::add_string): strdup(3) the `char *` argument and push it
      onto the `strings` stack before returning.
    
    Performance analysis
    ====================
    
    Slight improvement.
    
    src/devices/grohtml/post-html.cpp: Refactor.
    0.21    5.767   0.056391488719487
    
    [grohtml]: Fix memory leak.
    0.19    5.7315  0.04659737964171
---
 ChangeLog                         | 13 +++++++++++++
 src/devices/grohtml/post-html.cpp | 23 ++++++++++++++++++-----
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2d6439acc..98a042c5f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2026-06-03  G. Branden Robinson <[email protected]>
+
+       * src/devices/grohtml/post-html.cpp: Fix memory leak.
+       Preprocessor-include C++ "<stack>" header file.
+       (class char_buffer): Add new private member variable `strings`
+       of type `std::stack<char *>`, to keep track of the strdup(3)ped
+       pointers that our `add_string()` member functions return.
+       (char_buffer::char_buffer): Initialize `strings`.
+       (char_buffer::~char_buffer): Pop `strings` stack, free(3)ing
+       strdup(3)ped pointers until empty.
+       (char_buffer::add_string): strdup(3) the `char *` argument and
+       push it onto the `strings` stack before returning.
+
 2026-06-01  G. Branden Robinson <[email protected]>
 
        * src/devices/grohtml/post-html.cpp: Refactor.  New `static`
diff --git a/src/devices/grohtml/post-html.cpp 
b/src/devices/grohtml/post-html.cpp
index 6cd739892..6378d9b99 100644
--- a/src/devices/grohtml/post-html.cpp
+++ b/src/devices/grohtml/post-html.cpp
@@ -31,7 +31,7 @@ along with this program.  If not, see 
<http://www.gnu.org/licenses/>. */
 #include <errno.h>
 #include <stdio.h> // EOF, FILE, fclose(), fflush(), fopen(), freopen(),
                   // fseek(), SEEK_SET, setbuf(), stderr, stdout
-#include <stdlib.h> // abs(), atoi(), EXIT_SUCCESS, exit()
+#include <stdlib.h> // abs(), atoi(), EXIT_SUCCESS, exit(), free()
 #include <string.h> // strcmp(), strdup(), strerror(), strlen(),
                    // strncmp()
 #include <strings.h> // strcasecmp()
@@ -39,6 +39,7 @@ along with this program.  If not, see 
<http://www.gnu.org/licenses/>. */
 
 #include <getopt.h> // getopt_long()
 
+#include <stack>
 #include <vector>
 
 // libgroff
@@ -402,7 +403,15 @@ public:
                    unsigned int /* length */);
   char  *add_string(const string & /* s */);
 private:
+  // Simulate two views into the `char` buffer; one for the whole
+  // buffer, and one for the list of individual C strings populating it.
+  // It's a simulation because we support the latter by making _copies_
+  // of the string, which is inefficient.  A better approach would be to
+  // store indices from the start of `vec`, but doing that requires
+  // retyping in this class's users.
+  // TODO: Do the retyping and store the indices, not copies of strings.
   std::vector<char> vec;
+  std::stack<char *> strings;
 };
 
 // A block size of 4096 seems to run faster and with less variance than
@@ -413,10 +422,15 @@ static const size_t char_buffer_block_size = 4096;
 char_buffer::char_buffer()
 {
   vec = std::vector<char>(char_buffer_block_size, '\0');
+  strings = std::stack<char *>();
 }
 
 char_buffer::~char_buffer()
 {
+  while (!strings.empty()) {
+    free(strings.top());
+    strings.pop();
+  }
 }
 
 char *char_buffer::add_string (const char *s, unsigned int length)
@@ -428,10 +442,9 @@ char *char_buffer::add_string (const char *s, unsigned int 
length)
   for (size_t i = 0; i < length; i++)
     vec.push_back(s[i]);
   vec.push_back('\0');
-  // XXX: This strdup() likely leaks memory.
-  // TODO: Have `char_buffer` maintain a list of `char` pointers and
-  // free them in destructor?
-  return strdup(s);
+  char *p = strdup(s);
+  strings.push(p);
+  return p;
 }
 
 char *char_buffer::add_string (const string &s)

_______________________________________________
groff-commit mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/groff-commit

Reply via email to