"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: tog: don't mark incorrect base commit in nested log views
To:
Mark Jamsek <mark@jamsek.com>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 31 Dec 2024 14:51:40 +0100

Download raw body.

Thread
On Wed, Jan 01, 2025 at 12:13:05AM +1100, Mark Jamsek wrote:
> Mark Jamsek <mark@jamsek.com> wrote:
> > Mark Jamsek <mark@jamsek.com> wrote:
> > > 
> > > The second and third diffs contain two small albeit related fixes for
> > > bugs discovered during the process of writing the test: the first fixes
> > > a NULL pointer deref from passing a NULL s->last_displayed_entry to
> > > got_tree_entry_get_next() when scrolling down in a somewhat contrived
> > > tree view that is less than 4 lines in height; and the other diff clamps
> > > an s->selected assignment to 0 rather than becoming negative in the
> > > similarly contrived case of views less than their header and border
> > > lines in height (i.e., [2,5] depending on the view), which can result in
> > > bogus indexes displayed in the tree view header. Both diffs include a
> > > test covering these bugs: without the fixes, the added log test fails
> > > with a [-1/4] index displayed in the nested tree view header; and the
> > > tree test segfaults when scrolling down in the 3-line high tree view.
> > > 
> > > 
> > > -----------------------------------------------
> > > commit df7caa1a7ae745e41ecee2ae1d3420a536796715
> > > from: Mark Jamsek <mark@jamsek.dev>
> > > date: Tue Dec 31 12:45:28 2024 UTC
> > >  
> > >  fix NULL deref when scrolling small tog tree views down
> > >  
> > >  In the rare case a tree view is smaller than four lines in height,
> > >  the last_displayed_entry is NULL. Check this condition on a scroll
> > >  down event with j/down arrow or ^f/pgdn to guard a NULL got_tree_entry
> > >  from being passed to got_tree_entry_get_next() where it is dereferenced.
> > >  And add a test case covering this path.
> > >  
> > 
> > fix a pasto in this diff: revised diff below:
> 
> Let's try that again with the right fix this time :)

I believe I've got everything applied in the correct order.

This all makes sense and tests are passing. ok.