Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible missing breaks in switch statement in agrep_init #29

Open
ryandesign opened this issue Nov 26, 2023 · 0 comments
Open

Possible missing breaks in switch statement in agrep_init #29

ryandesign opened this issue Nov 26, 2023 · 0 comments

Comments

@ryandesign
Copy link

Is the lack of break; in the 'O' and 'M' cases here intentional? They're undocumented (see #28) so I don't know how they're intended to function.

agrep/agrep.c

Lines 2707 to 2713 in b7d180f

case 'O':
POST_FILTER = ON;
case 'M':
MULTI_OUTPUT = ON;
case 'Z': break; /* no-op: used by glimpse */

When I compile with clang with -Werror=implicit-fallthrough in CFLAGS I get:

agrep.c:2710:4: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
 2710 |                         case 'M':
      |                         ^
agrep.c:2710:4: note: insert '__attribute__((fallthrough));' to silence this warning
 2710 |                         case 'M':
      |                         ^
      |                         __attribute__((fallthrough)); 
agrep.c:2710:4: note: insert 'break;' to avoid fall-through
 2710 |                         case 'M':
      |                         ^
      |                         break; 
agrep.c:2713:4: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
 2713 |                         case 'Z': break;        /* no-op: used by glimpse */
      |                         ^
agrep.c:2713:4: note: insert 'break;' to avoid fall-through
 2713 |                         case 'Z': break;        /* no-op: used by glimpse */
      |                         ^
      |                         break; 

For both cases, please either add break; if its omission was unintentional or annotate the fallthrough if it was intentional (or indicate the intention in a comment in this issue so someone can submit a PR).

There are various syntaxes for how to annotate an intentional fallthrough depending on the compiler so if you are going that route you may need to define a macro for that. For example LLVM defines a macro like this (yours can probably be simpler since you can skip the C++ variants, however for older compilers you'll need to check first if __has_attribute exists):

#if defined(__cplusplus) && __cplusplus > 201402L && LLVM_HAS_CPP_ATTRIBUTE(fallthrough)
#define LLVM_FALLTHROUGH [[fallthrough]]
#elif LLVM_HAS_CPP_ATTRIBUTE(gnu::fallthrough)
#define LLVM_FALLTHROUGH [[gnu::fallthrough]]
#elif __has_attribute(fallthrough)
#define LLVM_FALLTHROUGH __attribute__((fallthrough))
#elif LLVM_HAS_CPP_ATTRIBUTE(clang::fallthrough)
#define LLVM_FALLTHROUGH [[clang::fallthrough]]
#else
#define LLVM_FALLTHROUGH
#endif
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant