Re: [PATCH] connected: use buffered I/O to talk to rev-list

From: Chris Torek <chris.torek@gmail.com>
Date: Aug 2, 2020, 16:08 UTC
Message-ID: <CAPx1Gvesmwf_CBj6D=YriFQgtEsDyUe7PHa6p9AvxG=fwAvHpA@mail.gmail.com>
In-Reply-To: <2e2907ac-3be9-c0ed-830a-f8aa28b471aa@web.de>

On Sun, Aug 2, 2020 at 7:39 AM René Scharfe <l.s.r@web.de> wrote:

Show 21 quoted lines

> @@ -135,16 +135,10 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
>                 if (new_pack && find_pack_entry_one(oid.hash, new_pack))
>                         continue;
>
> -               memcpy(commit, oid_to_hex(&oid), hexsz);
> -               if (write_in_full(rev_list.in, commit, hexsz + 1) < 0) {
> -                       if (errno != EPIPE && errno != EINVAL)
> -                               error_errno(_("failed write to rev-list"));
> -                       err = -1;
> -                       break;
> -               }
> +               fprintf(rev_list_in, "%s\n", oid_to_hex(&oid));
>         } while (!fn(cb_data, &oid));
>
> -       if (close(rev_list.in))
> +       if (fclose(rev_list_in))
>                 err = error_errno(_("failed to close rev-list's stdin"));
>
>         sigchain_pop(SIGPIPE);
> --
> 2.28.0

The same ferror()-before-fclose() remarks apply here too, but this time the explicit errno checking (EPIPE) cannot be done -- it's too late, errno is probably overwritten. I'm not sure how valuable the explicit errno tests are in the first place so I will leave that to others, but if we want to keep the explicit tests, use:

    if (fprintf(...) < 0)

to check each fprintf(), and add a final fflush() call (with another check) before the fclose().

Chris