From: Eric Sunshine <sunshine@sunshineco.com>
Date: Fri, 08 Feb 2019 21:40:28 GMT
Subject: Re: [PATCH v2] pack-refs: fail on falsely sorted packed-refs
Message-ID: <CAPig+cSNoXQQrDDXt6yN-gbYc3P4ZEiwJL1nwQWBomyLdVm_Vg@mail.gmail.com>
In-Reply-To: <20190208212221.31670-1-max@max630.net>

On Fri, Feb 8, 2019 at 4:22 PM Max Kirillov <max@max630.net> wrote:
> If packed-refs is marked as sorted but not really sorted it causes
> very hard to comprehend misbehavior of reference resolving - a reference
> is reported as not found, though it is listed by commands which output
> the references list.
>
> As the scope of the issue is not clear, make it visible by failing
> pack-refs command - the one which would not suffer performance penalty
> to verify the sortedness - when it encounters not really sorted existing
> data.
>
> Signed-off-by: Max Kirillov <max@max630.net>
> ---
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> @@ -1137,6 +1138,21 @@ static int write_with_updates(struct packed_ref_store *refs,
> +               if (iter)
> +               {
> +                       if (prev_ref.len &&  strcmp(prev_ref.buf, iter->refname) > 0)
> +                       {
> +                               [...]
> +                               strbuf_release(&prev_ref);
> +                               goto error;
> +                       }
> +
> +                       strbuf_init(&prev_ref, 0);
> +                       strbuf_addstr(&prev_ref, iter->refname);
> +               }

The call to strbuf_init() is leaking the allocated strbuf buffer each
time through the loop. The typical way to re-use a strbuf, and the way
you should do it here, is strbuf_reset().