Re: [PATCH 2/2] bundle: use a strbuf to scan the log for boundary commits

Junio C Hamano [off-list ref] writes:

Thomas Rast [off-list ref] writes:

quoted

  # v2 git bundle
  $(git rev-list --pretty=oneline --boundary <ARGS> | 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 [off-list ref]
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