Re: [PATCH 09/18] alternates: provide helper for adding to alternates list

From: Jacob Keller <jacob.keller@gmail.com>
Date: Oct 4, 2016, 06:07 UTC
Message-ID: <CA+P7+xq+NiCn53r+NCnPUysKW_V0semX4_1dGSN3OgMrJS2cJA@mail.gmail.com>
In-Reply-To: <20161003203503.omjwvg4ocz7pjyzt@sigill.intra.peff.net>

On Mon, Oct 3, 2016 at 1:35 PM, Jeff King <peff@peff.net> wrote:

Show 8 quoted lines

> The submodule code wants to temporarily add an alternate
> object store to our in-memory alt_odb list, but does it
> manually. Let's provide a helper so it can reuse the code in
> link_alt_odb_entry().
>
> While we're adding our new add_to_alternates_memory(), let's
> document add_to_alternates_file(), as the two are related.
>

Ya the code used in the submodule area always felt a bit wrong to me. It took me a bit to realize why we can just replace this all with a call to link_alt_odb_entry, but the resulting code reduction is definitely nice.

Show 18 quoted lines

> -       /* avoid adding it twice */
> -       prepare_alt_odb();
> -       for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
> -               if (alt_odb->name - alt_odb->base == objects_directory.len &&
> -                               !strncmp(alt_odb->base, objects_directory.buf,
> -                                       objects_directory.len))
> -                       goto done;
> -
> -       alloc = st_add(objects_directory.len, 42); /* for "12/345..." sha1 */
> -       alt_odb = xmalloc(st_add(sizeof(*alt_odb), alloc));
> -       alt_odb->next = alt_odb_list;
> -       xsnprintf(alt_odb->base, alloc, "%s", objects_directory.buf);
> -       alt_odb->name = alt_odb->base + objects_directory.len;
> -       alt_odb->name[2] = '/';
> -       alt_odb->name[40] = '\0';
> -       alt_odb->name[41] = '\0';
> -       alt_odb_list = alt_odb;
> -

Getting rid of multiple places for this funky extra allocation is a nice improvement.

Thanks, Jake

Show 9 quoted lines

> -       /* add possible alternates from the submodule */
> -       read_info_alternates(objects_directory.buf, 0);
> +       add_to_alternates_memory(objects_directory.buf);
>  done:
>         strbuf_release(&objects_directory);
>         return ret;
> --
> 2.10.0.618.g82cc264
>