Re: [PATCH v6 07/35] connect: convert get_remote_heads to use struct packet_reader

> On Tue, Mar 27, 2018 at 6:11 PM, Jeff King <peff@peff.net> wrote:
>> On Tue, Mar 27, 2018 at 05:27:14PM +0200, Duy Nguyen wrote:
>>
>>> On Thu, Mar 15, 2018 at 10:31:14AM -0700, Brandon Williams wrote:
>>> > In order to allow for better control flow when protocol_v2 is introduced
>>> > +static enum protocol_version discover_version(struct packet_reader *reader)
>>> > +{
>>> > +   enum protocol_version version = protocol_unknown_version;
>>> > +
>>> > +   /*
>>> > +    * Peek the first line of the server's response to
>>> > +    * determine the protocol version the server is speaking.
>>> > +    */
>>> > +   switch (packet_reader_peek(reader)) {
>>> > +   case PACKET_READ_EOF:
>>> > +           die_initial_contact(0);
>>> > +   case PACKET_READ_FLUSH:
>>>
>>> gcc is dumb. When -Werror and -Wimplicit-fallthrough are enabled (on
>>> at least gcc 7.x), it fails to realize that this die_initial_contact()
>>> will not fall through (even though we do tell it about die() not
>>> returning, but I guess that involves more flow analysis to realize
>>> die_initial_contact is in the same boat).
>>> [...]
>>> @@ -124,6 +124,7 @@ enum protocol_version discover_version(struct packet_reader *reader)
>>>       switch (packet_reader_peek(reader)) {
>>>       case PACKET_READ_EOF:
>>>               die_initial_contact(0);
>>> +             break;
>>
>> Would it make sense just to annotate that function to help the flow
>> analysis?
>
> Yes that works wonderfully with my gcc-7.3.0