Skip to content

Commit 1cf01a3

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d0e9983 commit 1cf01a3

File tree

13 files changed

+15
-4
lines changed

13 files changed

+15
-4
lines changed

apply.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2920,14 +2920,15 @@ static int apply_one_fragment(struct apply_state *state,
29202920
if (plen && (ws_rule & WS_BLANK_AT_EOF) &&
29212921
ws_blank_line(patch + 1, plen, ws_rule))
29222922
is_blank_context = 1;
2923+
/* fallthrough */
29232924
case '-':
29242925
memcpy(old, patch + 1, plen);
29252926
add_line_info(&preimage, old, plen,
29262927
(first == ' ' ? LINE_COMMON : 0));
29272928
old += plen;
29282929
if (first == '-')
29292930
break;
2930-
/* Fall-through for ' ' */
2931+
/* fallthrough */
29312932
case '+':
29322933
/* --no-add does not add new lines */
29332934
if (first == '+' && state->no_add)

builtin/cat-file.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
113113

114114
if (textconv_object(path, obj_context.mode, &oid, 1, &buf, &size))
115115
break;
116+
/* else fallthrough */
116117

117118
case 'p':
118119
type = sha1_object_info(oid.hash, NULL);

builtin/checkout.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
436436
* update paths in the work tree, and we cannot revert
437437
* them.
438438
*/
439+
/* fallthrough */
439440
case 0:
440441
return 0;
441442
default:

builtin/remote-ext.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ static char *strip_escapes(const char *str, const char *service,
5757
special = str[rpos];
5858
if (rpos == 1)
5959
break;
60-
/* Fall-through to error. */
60+
/* fallthrough */
6161
default:
6262
die("Bad remote-ext placeholder '%%%c'.",
6363
str[rpos]);

builtin/submodule--helper.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,6 +1189,7 @@ static int push_check(int argc, const char **argv, const char *prefix)
11891189
break;
11901190
die("HEAD does not match the named branch in the superproject");
11911191
}
1192+
/* fallthrough */
11921193
default:
11931194
die("src refspec '%s' must name a ref",
11941195
rs->src);

config.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,6 +2353,7 @@ static int store_write_pair(int fd, const char *key, const char *value)
23532353
case '"':
23542354
case '\\':
23552355
strbuf_addch(&sb, '\\');
2356+
/* fallthrough */
23562357
default:
23572358
strbuf_addch(&sb, value[i]);
23582359
break;

convert.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1545,8 +1545,9 @@ static int ident_filter_fn(struct stream_filter *filter,
15451545
switch (ident->state) {
15461546
default:
15471547
strbuf_add(&ident->left, head, ident->state);
1548+
/* fallthrough */
15481549
case IDENT_SKIPPING:
1549-
/* fallthru */
1550+
/* fallthrough */
15501551
case IDENT_DRAINING:
15511552
ident_drain(ident, &output, osize_p);
15521553
}

fsck.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,7 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)
588588
case S_IFREG | 0664:
589589
if (!options->strict)
590590
break;
591+
/* fallthrough */
591592
default:
592593
has_bad_modes = 1;
593594
}

http-push.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,6 +1523,7 @@ static int remote_exists(const char *path)
15231523
break;
15241524
case HTTP_ERROR:
15251525
error("unable to access '%s': %s", url, curl_errorstr);
1526+
/* fallthrough */
15261527
default:
15271528
ret = -1;
15281529
}

mailinfo.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -822,6 +822,7 @@ static void handle_filter(struct mailinfo *mi, struct strbuf *line)
822822
if (!handle_commit_msg(mi, line))
823823
break;
824824
mi->filter_stage++;
825+
/* fallthrough */
825826
case 1:
826827
handle_patch(mi, line);
827828
break;

0 commit comments

Comments
 (0)