consistently use "fallthrough" comments in switches

Gcc 7 adds -Wimplicit-fallthrough, which can warn when a
switch case falls through to the next case. The general idea
is that the compiler can't tell if this was intentional or
not, so you should annotate any intentional fall-throughs as
such, leaving it to complain about any unannotated ones.

There's a GNU __attribute__ which can be used for
annotation, but of course we'd have to #ifdef it away on
non-gcc compilers. Gcc will also recognize
specially-formatted comments, which matches our current
practice. Let's extend that practice to all of the
unannotated sites (which I did look over and verify that
they were behaving as intended).

Ideally in each case we'd actually give some reasons in the
comment about why we're falling through, or what we're
falling through to. And gcc does support that with
-Wimplicit-fallthrough=2, which relaxes the comment pattern
matching to anything that contains "fallthrough" (or a
variety of spelling variants). However, this isn't the
default for -Wimplicit-fallthrough, nor for -Wextra. In the
name of simplicity, it's probably better for us to support
the default level, which requires "fallthrough" to be the
only thing in the comment (modulo some window dressing like
"else" and some punctuation; see the gcc manual for the
complete set of patterns).

This patch suppresses all warnings due to
-Wimplicit-fallthrough. We might eventually want to add that
to the DEVELOPER Makefile knob, but we should probably wait
until gcc 7 is more widely adopted (since earlier versions
will complain about the unknown warning type).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/apply.c b/apply.c
index 71cbbd141c73bd72778d71e1e2400413c11f492f..c022af53a24bf56556d136a3456862cf6a2ca7cc 100644 (file)
--- a/apply.c
+++ b/apply.c
@@ -2920,6 +2920,7 @@ static int apply_one_fragment(struct apply_state *state,
                        if (plen && (ws_rule & WS_BLANK_AT_EOF) &&
                            ws_blank_line(patch + 1, plen, ws_rule))
                                is_blank_context = 1;
+                       /* fallthrough */
                case '-':
                        memcpy(old, patch + 1, plen);
                        add_line_info(&preimage, old, plen,
@@ -2927,7 +2928,7 @@ static int apply_one_fragment(struct apply_state *state,
                        old += plen;
                        if (first == '-')
                                break;
-               /* Fall-through for ' ' */
+                       /* fallthrough */
                case '+':
                        /* --no-add does not add new lines */
                        if (first == '+' && state->no_add)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 4ccbfaac3148287535aa8833d0d6cbdacb7c1d3d..aee280ea2cdb988ef918a2fb8fbdb3f80c68a251 100644 (file)
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -113,6 +113,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 
                if (textconv_object(path, obj_context.mode, &oid, 1, &buf, &size))
                        break;
+               /* else fallthrough */
 
        case 'p':
                type = sha1_object_info(oid.hash, NULL);

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5c202b7af57e26f5171eb8a83b107fceeb747488..d091f06274ca12c218ca9538ddee40dcd72dbe36 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -436,6 +436,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
                 * update paths in the work tree, and we cannot revert
                 * them.
                 */
+               /* fallthrough */
        case 0:
                return 0;
        default:

diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index bfb21ba7d217bafaf3527af007cd4ac4658d36c5..6a9127a33c019c5e6336075b5f68c7514d720b04 100644 (file)
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -57,7 +57,7 @@ static char *strip_escapes(const char *str, const char *service,
                                special = str[rpos];
                                if (rpos == 1)
                                        break;
-                               /* Fall-through to error. */
+                               /* fallthrough */
                        default:
                                die("Bad remote-ext placeholder '%%%c'.",
                                        str[rpos]);

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 818fe74f0af8696fe21a75d4aaf6b919b645d22b..f6a5e1af5d7c2fdb827f139e899e96cb9c4d91b5 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1189,6 +1189,7 @@ static int push_check(int argc, const char **argv, const char *prefix)
                                                break;
                                        die("HEAD does not match the named branch in the superproject");
                                }
+                               /* fallthrough */
                        default:
                                die("src refspec '%s' must name a ref",
                                    rs->src);

diff --git a/config.c b/config.c
index cd5a69e6308c0cad6028deb04fb17e8d2dc4086c..08490dfe81f369bc863635f34c275a343ba31db5 100644 (file)
--- a/config.c
+++ b/config.c
@@ -2353,6 +2353,7 @@ static int store_write_pair(int fd, const char *key, const char *value)
                case '"':
                case '\\':
                        strbuf_addch(&sb, '\\');
+                       /* fallthrough */
                default:
                        strbuf_addch(&sb, value[i]);
                        break;

diff --git a/convert.c b/convert.c
index a09935cb8123f04a8fdb4d48157c2a98e67b3c64..20d7ab67bdf889feb255e01a9b18cf4efa6107a3 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -1545,8 +1545,9 @@ static int ident_filter_fn(struct stream_filter *filter,
                switch (ident->state) {
                default:
                        strbuf_add(&ident->left, head, ident->state);
+                       /* fallthrough */
                case IDENT_SKIPPING:
-                       /* fallthru */
+                       /* fallthrough */
                case IDENT_DRAINING:
                        ident_drain(ident, &output, osize_p);
                }

diff --git a/fsck.c b/fsck.c
index 2d2d2e9432b96e5ff2ad893bd8661c0157d42eee..2ad00fc4d0356d8ec2cac21d1897a76b5002fd5b 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -588,6 +588,7 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
                case S_IFREG | 0664:
                        if (!options->strict)
                                break;
+                       /* fallthrough */
                default:
                        has_bad_modes = 1;
                }

diff --git a/http-push.c b/http-push.c
index e4c9b065ce13ca2c4e522e20f603f192ddcb9306..d860c477c60e7baec79220d9f06330a139d0e035 100644 (file)
--- a/http-push.c
+++ b/http-push.c
@@ -1523,6 +1523,7 @@ static int remote_exists(const char *path)
                break;
        case HTTP_ERROR:
                error("unable to access '%s': %s", url, curl_errorstr);
+               /* fallthrough */
        default:
                ret = -1;
        }

diff --git a/mailinfo.c b/mailinfo.c
index f2387a32675591d6da83473ef6235ba823721d1c..bcdbf98de57878a55c732e17b8baeb933f3cc056 100644 (file)
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -822,6 +822,7 @@ static void handle_filter(struct mailinfo *mi, struct strbuf *line)
                if (!handle_commit_msg(mi, line))
                        break;
                mi->filter_stage++;
+               /* fallthrough */
        case 1:
                handle_patch(mi, line);
                break;

diff --git a/quote.c b/quote.c
index 53b98a5b840d8fd4d73fab27348b657501923dca..de2922ddd63d6fc001822d116387c8ced7d7630d 100644 (file)
--- a/quote.c
+++ b/quote.c
@@ -431,6 +431,7 @@ void tcl_quote_buf(struct strbuf *sb, const char *src)
                case '{': case '}':
                case '$': case '\\': case '"':
                        strbuf_addch(sb, '\\');
+                       /* fallthrough */
                default:
                        strbuf_addch(sb, c);
                        break;

diff --git a/read-cache.c b/read-cache.c
index b211c57af6b418a24bb2eef97dabed0c95e53599..a051567610aab041017201f1b1bfa2e7e816a059 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -220,6 +220,7 @@ static int ce_modified_check_fs(const struct cache_entry *ce, struct stat *st)
        case S_IFDIR:
                if (S_ISGITLINK(ce->ce_mode))
                        return ce_compare_gitlink(ce) ? DATA_CHANGED : 0;
+               /* else fallthrough */
        default:
                return TYPE_CHANGED;
        }

diff --git a/send-pack.c b/send-pack.c
index b865f662e41880d11305ac6a4fe64450ef26f133..a8cc6b266e67847abafa613184752b3b781eb8dd 100644 (file)
--- a/send-pack.c
+++ b/send-pack.c
@@ -497,7 +497,7 @@ int send_pack(struct send_pack_args *args,
                                strbuf_release(&cap_buf);
                                return atomic_push_failure(args, remote_refs, ref);
                        }
-                       /* Fallthrough for non atomic case. */
+                       /* else fallthrough */
                default:
                        continue;
                }