Subject: Re: [PATCH 5/6] t0021/rot13-filter: add capability functions

From: Junio C Hamano <[email-masked]>
Date: Sun, 05 Nov 2017 11:03:20 +0900

Christian Couder <[email-masked]> writes:

>>> +             my ( $res, $buf ) = packet_bin_read();
>>> +             return ( $res, @cap ) if ( $res != 0 );
>>
>> The original had the same "'list eq list' does not do what you may
>> think it does" issue.  This one corrects it, which is good.
>>
>> I am not sure if ($res != 0) is correct though.  What should happen
>> when you get an unexpected EOF at this point?  The original would
>> have died; this ignores and continues.
>
> Well if there is an unexpected EOF, then packet_bin_read() returns
> (-1, ""), so packet_read_capabilities() returns (-1, @cap) where @cap
> contains the capabilities already received. Then
> packet_read_and_check_capabilities() checks that we received all the
> capabilities we expect and dies if that is not the case. If we did
> receive all the capabilities, then
> packet_read_and_check_capabilities() still returns -1, so the caller
> may check that and die.

In other words, it happens, by accident, to stop before going very
far.  I think we'd be better off making it an explicitly checked
error.

> But yeah we could also just die in packet_read_capabilities() if $res
> is -1. I will make this change.

Good.

Thanks.