From: Stefan Sperling <stsp@stsp.name>
Subject: Re: plug tog_view leaks on open_*_view() errors
To: Mark Jamsek <mark@jamsek.com>
Cc: Game of Trees <gameoftrees@openbsd.org>
Date: Fri, 14 Apr 2023 17:05:11 +0200

On Sat, Apr 15, 2023 at 12:50:49AM +1000, Mark Jamsek wrote:
> For each of the log, diff, blame, ref, and tree views, when their
> respective open_*_view() call errors, we call the corresponding
> close_*_view(), which releases view state. However, this does not
> deallocate the tog_view structure itself so it is leaked.
> 
> The below diff replaces the close_*_view() calls with view_close(),
> which also calls close_*_view() if the view->close function pointer has
> been initialised. If view->close has not been initialised, we also call
> close_*_view() too. This ensures both the view->state.* and tog_view
> structures are freed.
> 
> It might be easier understood by looking at the cmd_log() flow of
> events before this diff (which is the same for diff, tree, and ref;
> blame is slightly different but view is nonetheless leaked):
> 
>   view_open()
>     -> allocates view
>   open_log_view()
>     -> fails
>       -> close_log_view()
>         -> free view->state.log members
>       -> goto done
>   view leaked
> 
> The diff also includes a little whitespace cleanup which just happened
> to be where some related changes were made otherwise I'd split it out.

Nice catch. ok stsp