From: Jacob Keller <jacob.keller@gmail.com>
Date: Tue, 04 Oct 2016 06:12:19 GMT
Subject: Re: [PATCH 12/18] alternates: use a separate scratch space
Message-ID: <CA+P7+xqyuHHcMaUPajgEjt3Uzkcx6tEPEykuRaTPdvKSzt3jzg@mail.gmail.com>
In-Reply-To: <20161003203551.tmqp5rll6nqkewxz@sigill.intra.peff.net>

On Mon, Oct 3, 2016 at 1:35 PM, Jeff King <peff@peff.net> wrote:
> The alternate_object_database struct uses a single buffer
> both for storing the path to the alternate, and as a scratch
> buffer for forming object names. This is efficient (since
> otherwise we'd end up storing the path twice), but it makes
> life hard for callers who just want to know the path to the
> alternate. They have to remember to stop reading after
> "alt->name - alt->base" bytes, and to subtract one for the
> trailing '/'.
>
> It would be much simpler if they could simply access a
> NUL-terminated path string. We could encapsulate this in a
> function which puts a NUL in the scratch buffer and returns
> the string, but that opens up questions about the lifetime
> of the result. The first time another caller uses the
> alternate, the scratch buffer may get other data tacked onto
> it.
>
> Let's instead just store the root path separately from the
> scratch buffer. There aren't enough alternates being stored
> for the duplicated data to matter for performance, and this
> keeps things simple and safe for the callers.
>

Definitely agree here. The resulting code seems a lot easier to
follow, and making the callers simpler here is a very goo thing.

Thanks,
Jake