Re: [PATCH 08/16] t4200: use cut instead of sed

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 08/16] t4200: use cut instead of sed

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:22

Jeff King [off-list ref] writes:
quoted hunk
But the actual problem is that MERGE_RR lacks a trailing newline. I
don't see any code to add newlines, even though it seems possible that
we will write out several paths. So I think we need a newline here:
diff --git a/builtin-rerere.c b/builtin-rerere.c
index c607aad..e4a1dc1 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -58,7 +58,8 @@ static int write_rr(struct path_list *rr, int out_fd)
 		int length = strlen(path) + 1;
 		if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
 		    write_in_full(out_fd, "\t", 1) != 1 ||
-		    write_in_full(out_fd, path, length) != length)
+		    write_in_full(out_fd, path, length) != length ||
+		    write_in_full(out_fd, "\n", 1) != 1)
 			die("unable to write rerere record");
 	}
 	if (commit_lock_file(&write_lock) != 0)
No, check 3f43d72392b6c0477debd7edbd49bae9b7f41e60^:git-rerere.perl; the
records in this file are supposed to be NUL terminated (the paths are
allowed to have LF in them).
And unless I am missing something, rerere on multiple paths is very
broken (but that seems weird, since this code is so old).
I do not recall offhand an example of multi-path rerere working or not
working correctly, but I am not surprised if the C re-implementation is
broken.  Will need to check.

Re: [PATCH 08/16] t4200: use cut instead of sed

From: Jeff King <hidden>
Date: 2016-06-15 22:44:23

On Thu, Mar 13, 2008 at 11:00:23AM -0700, Junio C Hamano wrote:
quoted
index c607aad..e4a1dc1 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -58,7 +58,8 @@ static int write_rr(struct path_list *rr, int out_fd)
 		int length = strlen(path) + 1;
 		if (write_in_full(out_fd, rr->items[i].util, 40) != 40 ||
 		    write_in_full(out_fd, "\t", 1) != 1 ||
-		    write_in_full(out_fd, path, length) != length)
+		    write_in_full(out_fd, path, length) != length ||
+		    write_in_full(out_fd, "\n", 1) != 1)
 			die("unable to write rerere record");
 	}
 	if (commit_lock_file(&write_lock) != 0)
No, check 3f43d72392b6c0477debd7edbd49bae9b7f41e60^:git-rerere.perl; the
records in this file are supposed to be NUL terminated (the paths are
allowed to have LF in them).
Ah, OK. And it actually does do that correctly ("length = strlen(path) +
1"). So I think there isn't a bug there in rerere.

But if it's NUL-terminated, then using sed _definitely_ isn't portable
here. cut does work on Solaris in this case, but that might or might not
be portable to other systems with similar NUL problems. I'm not sure
what is the best route. We really just want to grab the sha1 from the
beginning of the first line. dd count=40? :)

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help