agrep icon indicating copy to clipboard operation
agrep copied to clipboard

Possible missing breaks in switch statement in agrep_init

Open ryandesign opened this issue 1 year ago • 0 comments

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.

https://github.com/Wikinaut/agrep/blob/b7d180fe73636740f694ec60c1ffab52b06e7150/agrep.c#L2707-L2713

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

ryandesign avatar Nov 26 '23 06:11 ryandesign