Re: [PATCH] quickfetch(): Prevent overflow of the rev-list command line

On Wednesday 08 July 2009, Johannes Sixt wrote:

Johan Herland schrieb:

quoted

+	/* If rev-list --stdin encounters an unknown commit, it
terminates, +	 * which will cause SIGPIPE in the write loop below.
*/

Under the conditions you describe here...

quoted

+	signal(SIGPIPE, SIG_IGN);

... and SIGPIPE being ignored...

quoted

+
+	err = start_command(&revlist);
+	if (err) {
+		error("could not run rev-list");
+		return err;
+	}

-	for (i = 0; argv[i]; i++)
-		free(argv[i]);
-	free(argv);
-	return err;
+	for (ref = ref_map; ref; ref = ref->next) {
+		if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) <
0 || +		    write_in_full(revlist.in, "\n", 1) < 0) {
+			error("failed write to rev-list");
+			err = errno;

... don't you get this error message with errno set to EPIPE?
Previously, there was no error message.

Indeed, you are correct. I guess the following should be added to the
patch:

 	if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) < 0 ||
 	    write_in_full(revlist.in, "\n", 1) < 0) {
-		error("failed write to rev-list");
-		err = errno;
+		if (errno != EPIPE) {
+			error("failed write to rev-list");
+			err = errno;
+		}
 		break;
 	}

Maybe I need to do something to the close() call as well? What happens on close() after EPIPE?


Thanks,

...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net