Re: [PATCH 1/3] refs: push lock management into packed backend

From: Han-Wen Nienhuys <hanwen@google.com>
Date: Sep 19, 2023, 15:52 UTC
Message-ID: <CAFQ2z_O-s-NT+DfxTSSuTq1Z6J0LZz8X8cFTPoOohfqX2rM-yw@mail.gmail.com>
In-Reply-To: <xmqqa5tjje0y.fsf@gitster.g>

On Tue, Sep 19, 2023 at 12:46 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 5 quoted lines

> This looks a bit more convoluted than necessary.  Is it the same as
>
>         if (refs->be->transaction_begin &&
>             refs->be->transaction_begin(refs, tr, err))
>                 FREE_AND_NULL(tr);

Changed this in the next version.

Show 10 quoted lines

> > +                     /* TODO: leaks on error path. */
> > +                     ref_transaction_free(packed_transaction);
> > +                     packed_transaction = NULL;
> > +                     backend_data->packed_transaction = NULL;
> > +             } else {
>
> If it were just a matter of flipping the early return and freeing of
> the transaction before going to clean-up, then that would have been
> less effort than leaving the TODO: comment.  What other things are
> needed to plug this leak?

you're not missing something. I didn't have enough time yesterday to look into all the details.

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Liana Sebastian