On Thu, Jul 9, 2009 at 10:37, Johan Herland<johan@xxxxxxxxxxx> wrote:
> On Thursday 09 July 2009, Alex Riesen wrote:
>> On Wed, Jul 8, 2009 at 18:01, Johan Herland<johan@xxxxxxxxxxx> wrote:
>> > On Wednesday 08 July 2009, Johannes Sixt wrote:
>> >> ... 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;
>>
>> You'll loose errno this way: error() does not save it.
>
> Not sure what you mean here. Should I move "err = errno;" outside the
> innermost "if"?
put it before error("failed write to rev-list"); or even before the
"if (err != EPIPE)".
Otherwise it is 0 after fprintf to stderr (which is the error() call).
> From my POV, if errno != EPIPE, we save it into err, and return that
> (overridden by finish_command()'s return value, if non-zero). If errno ==
> EPIPE, we're not interested in saving it, because we expect finish_command()
> to return non-zero in any case.
And you think this expectation makes the code simpler to understand?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html