| Message ID | 2e2907ac-3be9-c0ed-830a-f8aa28b471aa@web.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | connected: use buffered I/O to talk to rev-list | expand |
On Sun, Aug 2, 2020 at 7:39 AM René Scharfe <l.s.r@web.de> wrote: > @@ -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
Am 02.08.20 um 18:08 schrieb Chris Torek: > On Sun, Aug 2, 2020 at 7:39 AM René Scharfe <l.s.r@web.de> wrote: >> @@ -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); > > 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. The EPIPE check should really remain in the loop above. (Same comment on EPIPE on Windows applies here.) -- Hannes
diff --git a/connected.c b/connected.c index 937b4bae387..05c2916f38d 100644 --- a/connected.c +++ b/connected.c @@ -22,14 +22,13 @@ int check_connected(oid_iterate_fn fn, void *cb_data, struct check_connected_options *opt) { struct child_process rev_list = CHILD_PROCESS_INIT; + FILE *rev_list_in; struct check_connected_options defaults = CHECK_CONNECTED_INIT; - char commit[GIT_MAX_HEXSZ + 1]; struct object_id oid; int err = 0; struct packed_git *new_pack = NULL; struct transport *transport; size_t base_len; - const unsigned hexsz = the_hash_algo->hexsz; if (!opt) opt = &defaults; @@ -122,7 +121,8 @@ int check_connected(oid_iterate_fn fn, void *cb_data, sigchain_push(SIGPIPE, SIG_IGN); - commit[hexsz] = '\n'; + rev_list_in = xfdopen(rev_list.in, "w"); + do { /* * If index-pack already checked that: @@ -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);
Like f0bca72dc77 (send-pack: use buffered I/O to talk to pack-objects, 2016-06-08), significantly reduce the number of system calls and simplify the code for sending object IDs to rev-list by using stdio's buffering and handling errors after the loop. Signed-off-by: René Scharfe <l.s.r@web.de> --- connected.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) -- 2.28.0