tarantool / checkpatch

Checkpatch for Tarantool
GNU General Public License v2.0
2 stars 2 forks source link

There are no checks for missed breaks in switch/case and switch/default statements #62

Closed rtolkacheva closed 1 year ago

rtolkacheva commented 1 year ago

I saw a commit that contained switch/case and switch/default statements without break, return, or explicit FALLTHROUGH macro. The breaks should have been included but were accidentally omitted in the commit and were fixed later. Now there are two similar errors that the checkpatch can already find: PREFER_FALLTHROUGH and DEFAULT_NO_BREAK. Here are their descriptions from the documentation:

  **PREFER_FALLTHROUGH**
    Use the `FALLTHROUGH;` pseudo keyword instead of
    `/* fallthrough */` like comments.

  **DEFAULT_NO_BREAK**
    switch default case is sometimes written as "default:;".  This can
    cause new cases added below default to be defective.

    A "break;" should be added after empty default statement to avoid
    unwanted fallthrough.

Neither of them detects the possible error of forgotten breaks in switch/case and in non-empty switch/default. I see a possible solution: to check all switch/case and switch/default statements to ensure they end with break, return, or the FALLTHROUGH macro.

locker commented 1 year ago

Missing breaks in swtich-case statements can and should be detected by the compiler. gcc has the -Wimplicit-fallthrough flag that enables this check. This flag is enabled in the Tarantool main repository so there's no need to have this check implemented in checkpatch.