Re: [PATCH 1/3] archive: use packet_reader for communications

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

>> -	if (packet_read_line(fd[0], NULL))
>> +	status = packet_reader_read(&reader);
>> +	if (status != PACKET_READ_FLUSH)
>>  		die(_("git archive: expected a flush"));
>
> This makes me wonder what happens if we got an EOF instead.  We fail
> to notice protocol error here, but do the code after this part
> correctly handle the situation?

Sorry, this part of my comment is completely backwards.

We require they send a flush, not a 0-length data packet of length
4, and otherwise we die, even though we used to treate 0-length data
packet of length 4 just like a flush.

So this is making the code more strict than the original.  As long
as all the existing implementations correctly use flush here, there
would be no unintended regression, but it bothers me that we have to
even worry if these behaviour changes affect the already deployed
software negatively.

>
>>  	/* Now, start reading from fd[0] and spit it out to stdout */