On 03/27, Duy Nguyen wrote:

On Tue, Mar 27, 2018 at 6:25 PM, Duy Nguyen [off-list ref] wrote:

quoted

On Tue, Mar 27, 2018 at 6:11 PM, Jeff King [off-list ref] wrote:

quoted

On Tue, Mar 27, 2018 at 05:27:14PM +0200, Duy Nguyen wrote:

quoted

On Thu, Mar 15, 2018 at 10:31:14AM -0700, Brandon Williams wrote:

quoted

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