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