From f0bca72dc77f62d61fc355bd6fe6e32b194950b8 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 8 Jun 2016 15:42:16 -0400 Subject: [PATCH] send-pack: use buffered I/O to talk to pack-objects We start a pack-objects process and then write all of the positive and negative sha1s to it over a pipe. We do so by formatting each item into a fixed-size buffer and then writing each individually. This has two drawbacks: 1. There's some manual computation of the buffer size, which is not immediately obvious is correct (though it is). 2. We write() once per sha1, which means a lot more system calls than are necessary. We can solve both by wrapping the pipe descriptor in a stdio handle; this is the same technique used by upload-pack when serving fetches. Note that we can also simplify and improve the error handling here. The original detected a single write error and broke out of the loop (presumably to avoid writing the error message over and over), but never actually acted on seeing an error; we just fed truncated input and took whatever pack-objects returned. In practice, this probably didn't matter, as the likely errors would be caused by pack-objects dying (and we'd probably just die with SIGPIPE anyway). But we can easily make this simpler and more robust; the stdio handle keeps an error flag, which we can check at the end. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- send-pack.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/send-pack.c b/send-pack.c index 37ee04ea3..299d30384 100644 --- a/send-pack.c +++ b/send-pack.c @@ -36,18 +36,15 @@ int option_parse_push_signed(const struct option *opt, die("bad %s argument: %s", opt->long_name, arg); } -static int feed_object(const unsigned char *sha1, int fd, int negative) +static void feed_object(const unsigned char *sha1, FILE *fh, int negative) { - char buf[42]; - if (negative && !has_sha1_file(sha1)) - return 1; + return; - memcpy(buf + negative, sha1_to_hex(sha1), 40); if (negative) - buf[0] = '^'; - buf[40 + negative] = '\n'; - return write_or_whine(fd, buf, 41 + negative, "send-pack: send refs"); + putc('^', fh); + fputs(sha1_to_hex(sha1), fh); + putc('\n', fh); } /* @@ -73,6 +70,7 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru NULL, }; struct child_process po = CHILD_PROCESS_INIT; + FILE *po_in; int i; i = 4; @@ -97,21 +95,22 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru * We feed the pack-objects we just spawned with revision * parameters by writing to the pipe. */ + po_in = xfdopen(po.in, "w"); for (i = 0; i < extra->nr; i++) - if (!feed_object(extra->sha1[i], po.in, 1)) - break; + feed_object(extra->sha1[i], po_in, 1); while (refs) { - if (!is_null_oid(&refs->old_oid) && - !feed_object(refs->old_oid.hash, po.in, 1)) - break; - if (!is_null_oid(&refs->new_oid) && - !feed_object(refs->new_oid.hash, po.in, 0)) - break; + if (!is_null_oid(&refs->old_oid)) + feed_object(refs->old_oid.hash, po_in, 1); + if (!is_null_oid(&refs->new_oid)) + feed_object(refs->new_oid.hash, po_in, 0); refs = refs->next; } - close(po.in); + fflush(po_in); + if (ferror(po_in)) + die_errno("error writing to pack-objects"); + fclose(po_in); if (args->stateless_rpc) { char *buf = xmalloc(LARGE_PACKET_MAX); -- 2.11.0