Opened 8 weeks ago

Last modified 8 weeks ago

#32579 reopened enhancement

Use clang's -Wimplicit-fallthrough and fallthrough attribute on case statements

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: code-correctness
Cc: Actual Points:
Parent ID: Points: 1
Reviewer: Sponsor:

Description

We can avoid accidental fallthroughs using clang's -Wimplicit-fallthrough.

https://clang.llvm.org/docs/AttributeReference.html#fallthrough

Child Tickets

Change History (4)

comment:1 Changed 8 weeks ago by teor

Here's how we can check for that feature in clang:
https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros

I think the same syntax works on gcc, but we should check.

Last edited 8 weeks ago by teor (previous) (diff)

comment:2 Changed 8 weeks ago by nickm

Resolution: worksforme
Status: newclosed

I believe we already enable that -- see configure.ac, and see "warning_flags" when building with clang.

Please reopen if I'm wrong.

comment:3 Changed 8 weeks ago by teor

Resolution: worksforme
Status: closedreopened

If we are doing it, we're doing it wrong, here's a fallthrough that should require the fallthrough attribute:

src/core/proto/proto_socks.c-      case SOCKS_RESULT_TRUNCATED:
src/core/proto/proto_socks.c-        if (datalen == n_pullup)
src/core/proto/proto_socks.c-          return 0;
src/core/proto/proto_socks.c-        /* FALLTHRU */
src/core/proto/proto_socks.c-      case SOCKS_RESULT_MORE_EXPECTED:
src/core/proto/proto_socks.c-        res = 0;
src/core/proto/proto_socks.c-        break;

I don't think comments count, and even if they do, I can't imagine this spelling being supported :-)

Note: See TracTickets for help on using tickets.