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? Like:
diff --git a/connect.c b/connect.c
index c3a014c5ba..49eca46462 100644
--- a/connect.c
+++ b/connect.c
@@ -46,7 +46,7 @@ int check_ref_type(const struct ref *ref, int flags)
return check_ref(ref->name, flags);
}
-static void die_initial_contact(int unexpected)
+static NORETURN void die_initial_contact(int unexpected)
{
if (unexpected)
die(_("The remote end hung up upon initial contact"));
That should let the callers know what's going on, and inside the
function itself, the compiler should confirm that all code paths hit
another NORETURN function.
-Peff
