Re: [PATCH 2/8] builtin/multi-pack-index.c: support --stdin-packs mode

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 12, 2021, 02:08 UTC
Message-ID: <CAPig+cRAKJyzuMwPNdmC+BobrMjk5TF-o3Ort8R0E77ij=g6_A@mail.gmail.com>
In-Reply-To: <YTzYYfBGqG87O5mN@nand.local>

On Sat, Sep 11, 2021 at 12:25 PM Taylor Blau <me@ttaylorr.com> wrote:

Show 24 quoted lines

> On Sat, Sep 11, 2021 at 12:05:05PM +0200, Ævar Arnfjörð Bjarmason wrote:
> > Before calling string_list_clear(). I.e. we didn't strdup(), but during
> > free() we pretend that we did, because we did, just not in
> > string_list_append().
>
> Good catch. It's kind of gross, but the result is:
>
>  static void read_packs_from_stdin(struct string_list *to)
>  {
> -       struct strbuf buf = STRBUF_INIT;
> +       struct strbuf buf = STRBUF_INIT_NODUP;
>         while (strbuf_getline(&buf, stdin) != EOF) {
>                 string_list_append(to, strbuf_detach(&buf, NULL));
>         }
> @@ -107,6 +107,11 @@ static int cmd_multi_pack_index_write(int argc, const char **argv)
>                 ret = write_midx_file_only(opts.object_dir, &packs,
>                                            opts.preferred_pack, opts.flags);
>
> +               /*
> +                * pretend strings are duplicated to free the memory allocated
> +                * by read_packs_from_stdin()
> +                */
> +               packs.strdup_strings = 1;
>                 string_list_clear(&packs, 0);

An alternative is to do this:

    struct strbuf buf = STRBUF_INIT;
    ...
    while (...) {
        string_list_append_nodup(to, strbuf_detach(&buf, NULL));
        ...
    }
    ...
    string_list_clear(&packs, 0);

That is, use string_list_append_nodup(), which is documented as existing precisely for this sort of use-case:

    Like string_list_append(), except string is never copied.  When
    list->strdup_strings is set, this function can be used to hand
    ownership of a malloc()ed string to list without making an extra
    copy.

(I mention this only for completeness but don't care strongly which approach is used.)