bug-groff
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug #66269] [troff] double free when .IP indentation is less than the l


From: G. Branden Robinson
Subject: [bug #66269] [troff] double free when .IP indentation is less than the length of the tag
Date: Sat, 28 Sep 2024 23:43:18 -0400 (EDT)

Update of bug #66269 (group groff):

                  Status:               Need Info => Duplicate              
             Open/Closed:                    Open => Closed                 

    _______________________________________________________

Follow-up Comment #2:

Confirmed.  Duplicate of bug #65894.


b5513742757588e0cf9a043d16dbf06a424906bc is the first new commit
commit b5513742757588e0cf9a043d16dbf06a424906bc
Author: G. Branden Robinson <g.branden.robinson@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.

 ChangeLog               | 34 ++++++++++++++++++
 src/roff/troff/mtsm.cpp | 94
++++++++++++++++++-------------------------------
 src/roff/troff/mtsm.h   | 23 +++++-------
 3 files changed, 77 insertions(+), 74 deletions(-)




    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?66269>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]