On 03/27, Duy Nguyen wrote: > On Tue, Mar 27, 2018 at 6:25 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: > > On Tue, Mar 27, 2018 at 6:11 PM, Jeff King <peff@xxxxxxxx> 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 > > And this changes things. Since this series is 35 patches and there's > no sign of reroll needed, I'm going to make this change separately. > Don't reroll just because of this > -- > Duy Looks like a good change, but yes, it should work fine as a patch on top. -- Brandon Williams