Re: [PATCH 05/30] loose: add a mapping between SHA-1 and SHA-256 for loose objects

> Eric Sunshine <sunshine@sunshineco.com> writes:
> > On Wed, Sep 27, 2023 at 3:56 PM Eric W. Biederman <ebiederm@gmail.com> wrote:
> >> +       for (; iter != kh_end(map); iter++) {
> >> +               if (kh_exist(map, iter)) {
> >> +                       if (oideq(&kh_key(map, iter), the_hash_algo->empty_tree) ||
> >> +                           oideq(&kh_key(map, iter), the_hash_algo->empty_blob))
> >> +                               continue;
> >> +                       strbuf_addf(&buf, "%s %s\n", oid_to_hex(&kh_key(map, iter)), oid_to_hex(kh_value(map, iter)));
> >> +                       if (write_in_full(fd, buf.buf, buf.len) < 0)
> >> +                               goto errout;
> >> +                       strbuf_reset(&buf);
> >> +               }
> >> +       }
> >
> > Nit: If you call strbuf_reset() immediately before strbuf_addf(), then
> > you save the reader of the code the effort of having to scan backward
> > through the function to verify that `buf` is empty the first time
> > through the loop.
>
> I am actually perplexed why the code works this way.
>
> My gut says we should create the entire buffer in memory and then
> write it to disk all in a single system call, and not perform
> this line buffering.

> Doing that would remove the need for the strbuf_reset entirely,
> and would just require the buffer to be primed with the
> loose_object_header.
>
> But what is there works and I will leave it for now.