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).
Since -Wimplicit-fallthrough may be useful to spot bugs elsewhere and
there are only two places in this series that tick it off, is it
possible to squash in something like this? On the off chance that we
fail horribly to die, "break;" would stop the wrong code from
executing even more.
This covers another patch too, but you get the idea.
diff --git a/connect.c b/connect.c
index 54971166ac..847bf2f7d6 100644
--- a/connect.c
+++ b/connect.c
@@ -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;
case PACKET_READ_FLUSH:
case PACKET_READ_DELIM:
version = protocol_v0;
@@ -303,6 +304,7 @@ struct ref **get_remote_heads(struct packet_reader *reader,
switch (packet_reader_read(reader)) {
case PACKET_READ_EOF:
die_initial_contact(1);
+ break;
case PACKET_READ_NORMAL:
len = reader->pktlen;
if (len > 4 && skip_prefix(reader->line, "ERR ", &arg))
--
Duy
