From: Thomas Rast Date: Wed, 22 Feb 2012 20:25:53 GMT Subject: Re: [PATCH 2/2] bundle: use a strbuf to scan the log for boundary commits Message-ID: <87hayivcmm.fsf@thomas.inf.ethz.ch> In-Reply-To: <7vlinuaaab.fsf@alter.siamese.dyndns.org> Junio C Hamano writes: > Thomas Rast writes: > >> # v2 git bundle >> $(git rev-list --pretty=oneline --boundary | grep ^-) >> >> git-bundle actually spawns exactly this rev-list invocation, and does >> the grepping internally. >> >> There was a subtle bug in the latter step: it used fgets() with a >> 1024-byte buffer. If the user has sufficiently long subjects (e.g., >> by not adhering to the git oneline-subject convention in the first >> place), the 'oneline' format can easily overflow the buffer. > > Thanks for diagnosing this, but I wonder if it even needs --pretty=oneline > to begin with, except for debugging purposes. > > Do we ever use the subject string read from the rev-list output in any > way? > > In other words, I am wondering if the right patch to minimally fix the > issue starting from older releases is something along this line instead: Not sure. The only use I could think of would be to google for the subjects, in the hope of finding some repository that has the commit you are looking for. Other than that... In any case the --pretty=oneline is very deliberate, as we can see from the commit below. It just doesn't give a reason :-) commit 239296770dae75e21c179733785731ec6ffae1f5 Author: Johannes Schindelin Date: Fri Feb 23 03:17:51 2007 +0100 git-bundle: record commit summary in the prerequisite data diff --git a/builtin-bundle.c b/builtin-bundle.c index 191ec55..d74afaa 100644 --- a/builtin-bundle.c +++ b/builtin-bundle.c @@ -267,7 +267,7 @@ static int create_bundle(struct bundle_header *header, const char *path, int argc, const char **argv) { int bundle_fd = -1; - const char **argv_boundary = xmalloc((argc + 3) * sizeof(const char *)); + const char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *)); const char **argv_pack = xmalloc(4 * sizeof(const char *)); int pid, in, out, i, status; char buffer[1024]; @@ -282,10 +282,11 @@ static int create_bundle(struct bundle_header *header, const char *path, write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature)); /* write prerequisites */ - memcpy(argv_boundary + 2, argv + 1, argc * sizeof(const char *)); + memcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *)); argv_boundary[0] = "rev-list"; argv_boundary[1] = "--boundary"; - argv_boundary[argc + 1] = NULL; + argv_boundary[2] = "--pretty=oneline"; + argv_boundary[argc + 2] = NULL; out = -1; pid = fork_with_pipe(argv_boundary, NULL, &out); if (pid < 0) -- Thomas Rast trast@{inf,student}.ethz.ch