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

Johan Herland schrieb:

However, using rev-list --stdin is not entirely straightforward: rev-list
terminates immediately when encountering an unknown object, which can
trigger SIGPIPE if we are still writing object's to its standard input.
We therefore ignore SIGPIPE so that the fetch process is not terminated.

I removed the "signal(SIGPIPE, SIG_IGN)", but the test suite still passes.
IOW, there is no test case that has the configuration that you describe
here. Would you please add such a test (perhaps in t5502)? It would also
help me verify the patch works as intended on Windows.

Signed-off-by: Johan Herland <redacted>
Improved-by: Johannes Sixt [off-list ref]

Please make this [off-list ref] despite the email address I'm using right now.

Improved-by: Alex Riesen [off-list ref]
Tested-by: Peter Krefting <redacted>

+	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) {
+			err = errno;
+			if (err != EPIPE && err != EINVAL)
+				error("failed write to rev-list");
+			break;
+		}
+	}
+
+	if (close(revlist.in)) {
+		err = errno;
+		error("failed to close rev-list's stdin");
+	}
+	return finish_command(&revlist) || err;

The call site of quickfetch() is not interested in the errno, only on
whether the return value is non-zero: You can just assign -1 to err
(that's our convention for failure). OTOH, it would be helpful to include
strerror(errno) in the error message.

Shouldn't you reset signal(SIGPIPE) to its previous value?

-- Hannes