gbranden pushed a commit to branch master
in repository groff.
commit ec6e93c7368e823eaa8b44084730c9bbf4934a83
Author: G. Branden Robinson <[email protected]>
AuthorDate: Sun Apr 13 13:42:53 2025 -0500
[troff]: Refactor `grostream` for memory safety.
* src/roff/troff/input.cpp: Refactor `grostream` class for memory
safety.
(struct grosteam): Convert from this...
(class grostream): ...to this, inheriting from class `object` so that
it casts less dangerously thence and thither when stashing
`grostream`s into and retrieving them from `object_dictionary`
objects.
(class grostream): Also re-type `filename` and `mode` member variables
from (groff) `string` to `symbol`, since they won't have embedded null
characters (and take up half as much space; `sizeof (string)` is 16 on
GNU/Linux amd64 whereas that of `symbol` is 8). Making this change by
itself exposed memory corruption problems, resolved by resituating
`grostream` in the class hierarchy.
(grostream::grostream): Drop now unnecessary explicit null
termination. That property is implied by the `symbol` class.
(open_file): Make newly created object anonymous, and drop frightening
C-style typecast. Now a `grostream` casts safely and implicitly to
and from `object`.
Problem likely introduced by me in commit 6d32f2492, 24 September, but
it was latent (requiring the `symbol` change noted above), and without
JSON dumpers for `string` and `symbol` objects, visibility into memory
mischief was limited if it didn't cause a SEGV on a developer's
machine.
---
ChangeLog | 28 ++++++++++++++++++++++++++++
src/roff/troff/input.cpp | 20 +++++++++-----------
2 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ef38d0767..e72b04435 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,31 @@
+2025-04-13 G. Branden Robinson <[email protected]>
+
+ * src/roff/troff/input.cpp: Refactor `grostream` class for
+ memory safety.
+ (struct grosteam): Convert from this...
+ (class grostream): ...to this, inheriting from class `object` so
+ that it casts less dangerously thence and thither when stashing
+ `grostream`s into and retrieving them from `object_dictionary`
+ objects.
+ (class grostream): Also re-type `filename` and `mode` member
+ variables from (groff) `string` to `symbol`, since they won't
+ have embedded null characters (and take up half as much space;
+ `sizeof (string)` is 16 on GNU/Linux amd64 whereas that of
+ `symbol` is 8). Making this change by itself exposed memory
+ corruption problems, resolved by resituating `grostream` in the
+ class hierarchy.
+ (grostream::grostream): Drop now unnecessary explicit null
+ termination. That property is implied by the `symbol` class.
+ (open_file): Make newly created object anonymous, and drop
+ frightening C-style typecast. Now a `grostream` casts safely
+ and implicitly to and from `object`.
+
+ Problem likely introduced by me in commit 6d32f2492, 24
+ September, but it was latent (requiring the `symbol` change
+ noted above), and without JSON dumpers for `string` and `symbol`
+ objects, visibility into memory mischief was limited if it
+ didn't cause a SEGV on a developer's machine.
+
2025-04-13 G. Branden Robinson <[email protected]>
* src/libs/libgroff/symbol.cpp: Fix code style nits.
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index e30b3f751..eb015a5b3 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -7605,20 +7605,18 @@ void terminal_continue()
do_terminal(0, 1);
}
-struct grostream {
- string filename;
- string mode;
+class grostream : public object {
+public:
+ symbol filename;
+ symbol mode;
FILE *file;
- grostream(const string &fn, string m, FILE *fp);
+ grostream(const char *fn, symbol m, FILE *fp);
~grostream();
};
-grostream::grostream(const string &fn, string m, FILE *fp)
+grostream::grostream(const char *fn, symbol m, FILE *fp)
: filename(fn), mode(m), file(fp)
{
- // Don't leak garbage in print_streams().
- filename += '\0';
- mode += '\0';
}
// XXX: Maybe we should try to close the libc FILE stream here.
@@ -7663,7 +7661,7 @@ static void open_file(bool appending)
if (!stream.is_null()) {
char *filename = read_rest_of_line_as_argument();
if (filename != 0 /* nullptr */) {
- const string mode = appending ? "appending" : "writing";
+ const char *mode = appending ? "appending" : "writing";
errno = 0;
FILE *fp = fopen(filename, appending ? "a" : "w");
if (0 /* nullptr */ == fp) {
@@ -7686,8 +7684,8 @@ static void open_file(bool appending)
return;
}
}
- grostream *grost = new grostream(filename, mode, &*fp);
- stream_dictionary.define(stream, (object *)grost);
+ stream_dictionary.define(stream,
+ new grostream(filename, mode, &*fp));
}
}
// TODO: Add `filename` to file name set.
_______________________________________________
groff-commit mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/groff-commit
