Update of bug #65894 (group groff): Status: In Progress => Fixed Open/Closed: Open => Closed Planned Release: None => 1.24.0
_______________________________________________________ Follow-up Comment #1: This was a can of worms. Here are the resolving commits in forward chronological order. commit aab74ead30a3356252011314a2af1642e36653ad Author: G. Branden Robinson <g.branden.robin...@gmail.com> Date: Thu Jun 20 11:59:06 2024 -0500 [groff]: Add regression test for latent troff bug. Add regression test for latent bug: surprising "grout" produced when using "html" output device and a memory management bug is fixed. * src/roff/groff/tests/html-does-not-fumble-tagged-paragraph.sh: Do it. * src/roff/groff/groff.am (groff_TESTS): Run test. commit b5513742757588e0cf9a043d16dbf06a424906bc Author: G. Branden Robinson <g.branden.robin...@gmail.com> Date: Wed Jun 19 10:38:43 2024 -0500 [troff]: Fix Savannah #65894. Refactor troff's "mini-troff state machine" to use STL stack implementation instead of a bespoke one, fixing a double-free error. * src/roff/troff/mtsm.h: Include C++ standard library "stack" header. (struct stack): Drop. (class mtsm): Declare `stack` data member as a standard stack of `statem` objects instead of a pointer to local `stack` type. * src/roff/troff/mtsm.h (statem::merge, statem::update): * src/roff/troff/mtsm.cpp (statem::merge, statem::update): Take references instead of pointers to some `statem` arguments (those that will be accessed via the `stack` data member). * src/roff/troff/mtsm.cpp (stack::stack, stack::~stack): Drop. (mtsm::mtsm): Drop initializer of `sp` data member. (mtsm::~mtsm): Drop (conditional) deletion of `sp`. (mtsm::push_state, mtsm::pop_state, mtsm::inherit): Use STL `stack`'s `push()`, `empty()`, and `pop()` instead of primitive operations. (statem::update): Access data members of `mtsm` class via references instead of pointers (using `.` instead of `->` operator). (statem::merge): Drop null pointer test of reference, which can't be null. Fixes <https://savannah.gnu.org/bugs/?65894>. This was a latent bug exposed by commit 0951ff53e4, 10 August (a change to the man(7) macro package; such things should _never_ cause the formatter to crash). Also drop old-style Emacs file-local variable setting. The following test fails at this commit: src/roff/groff/tests/html-does-not-fumble-tagged-paragraph.sh I attempted a less intrusive change, but did not succeed. The copying of pointers to groff strings as data members of objects using C++ default copy constructors that also get automatically cleaned up by destructors also worried me--and the MTSM logic does this sort of thing a lot--but delegating stack handling to the STL suffices to resolve this concrete problem. I continue to nurse reservations about the wisdom of attempting to infer HTML structure from troff requests not designed for that purpose, the necessity of state management that this approach imposes, and the robustness of our implementation, which we documented as being "beta code" about 25 years ago and have changed little since. I welcome objections to and arguments against my opinion if they're accompanied by code changes that improve the situation. commit 2cfc5ddeec5d74109818cfefd7f3f29354f29545 Author: G. Branden Robinson <g.branden.robin...@gmail.com> Date: Thu Jun 20 12:36:16 2024 -0500 [troff]: Fix failing test. * src/roff/troff/mtsm.cpp (mtsm::inherit): Fix failing test. I freely admit I don't completely understand why this works. But it restores correct man page formatting. Complicating the problem is that grohtml/MTSM's support for centering and right-alignment (the `ce` and `rj` requests) never really worked anyway. Try this with any version of groff supporting grohtml: $ cat ATTIC/ce-rj-html.groff .ce 1 This is centered. This is not. .rj 1 This is right-aligned. This is aligned to the left. $ ~/groff-1.22.3/bin/groff -Tascii ./ATTIC/ce-rj-html.groff | cat -s This is centered. This is not. This is right-aligned. This is aligned to the left. $ ~/groff-1.22.3/bin/groff -Thtml ./ATTIC/ce-rj-html.groff | grep '<p' <p align="center">This is centered. This is not.</p> <p>This is right-aligned. This is aligned to the left.</p> Why were these features (in grohtml/MTSM) put in if they didn't work? _______________________________________________________ Reply to this item at: <https://savannah.gnu.org/bugs/?65894> _______________________________________________ Message sent via Savannah https://savannah.gnu.org/