From 25f1fbdc72d7b8266525aaf59933d284cc412406 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Fri, 13 Sep 2024 02:23:25 -0700 Subject: [PATCH] fix some linting errors noted by clang-tidy (#999) - enable generation of compilation database for all non-Arduino builds - add clang-tidy config - resolve implicit casting cases - resolve `else` after `return` cases - ignore false-negative about "memory allocated with zero size" --- .clang-tidy | 154 ++++++++++++++++++++++++++++++++++ CMakeLists.txt | 3 + RF24.cpp | 38 +++------ RF24.h | 6 +- RF24_config.h | 10 +-- examples_linux/CMakeLists.txt | 3 + examples_pico/CMakeLists.txt | 3 + printf.h | 6 +- pyRF24/pyRF24.cpp | 10 +-- 9 files changed, 191 insertions(+), 42 deletions(-) create mode 100644 .clang-tidy diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 000000000..adb03ecb1 --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,154 @@ +--- +Checks: 'clang-diagnostic-*,clang-analyzer-*,bugprone-*,-bugprone-easily-swappable-parameters,clang-diagnostics-*,clang-analyzer-*' +WarningsAsErrors: '' +HeaderFilterRegex: '' +AnalyzeTemporaryDtors: false +FormatStyle: file +User: brendan +CheckOptions: + - key: bugprone-string-constructor.LargeLengthThreshold + value: '8388608' + - key: modernize-replace-auto-ptr.IncludeStyle + value: llvm + - key: bugprone-reserved-identifier.Invert + value: 'false' + - key: bugprone-implicit-widening-of-multiplication-result.UseCXXStaticCastsInCppSources + value: 'true' + - key: cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField + value: 'false' + - key: bugprone-unused-return-value.CheckedFunctions + value: '::std::async;::std::launder;::std::remove;::std::remove_if;::std::unique;::std::unique_ptr::release;::std::basic_string::empty;::std::vector::empty;::std::back_inserter;::std::distance;::std::find;::std::find_if;::std::inserter;::std::lower_bound;::std::make_pair;::std::map::count;::std::map::find;::std::map::lower_bound;::std::multimap::equal_range;::std::multimap::upper_bound;::std::set::count;::std::set::find;::std::setfill;::std::setprecision;::std::setw;::std::upper_bound;::std::vector::at;::bsearch;::ferror;::feof;::isalnum;::isalpha;::isblank;::iscntrl;::isdigit;::isgraph;::islower;::isprint;::ispunct;::isspace;::isupper;::iswalnum;::iswprint;::iswspace;::isxdigit;::memchr;::memcmp;::strcmp;::strcoll;::strncmp;::strpbrk;::strrchr;::strspn;::strstr;::wcscmp;::access;::bind;::connect;::difftime;::dlsym;::fnmatch;::getaddrinfo;::getopt;::htonl;::htons;::iconv_open;::inet_addr;::isascii;::isatty;::mmap;::newlocale;::openat;::pathconf;::pthread_equal;::pthread_getspecific;::pthread_mutex_trylock;::readdir;::readlink;::recvmsg;::regexec;::scandir;::semget;::setjmp;::shm_open;::shmget;::sigismember;::strcasecmp;::strsignal;::ttyname' + - key: cert-dcl16-c.NewSuffixes + value: 'L;LL;LU;LLU' + - key: bugprone-exception-escape.FunctionsThatShouldNotThrow + value: '' + - key: modernize-loop-convert.MaxCopySize + value: '16' + - key: bugprone-narrowing-conversions.WarnOnFloatingPointNarrowingConversion + value: 'true' + - key: bugprone-signed-char-misuse.CharTypdefsToIgnore + value: '' + - key: bugprone-argument-comment.CommentStringLiterals + value: '0' + - key: bugprone-narrowing-conversions.PedanticMode + value: 'false' + - key: bugprone-sizeof-expression.WarnOnSizeOfConstant + value: 'true' + - key: bugprone-assert-side-effect.IgnoredFunctions + value: __builtin_expect + - key: bugprone-argument-comment.CommentBoolLiterals + value: '0' + - key: bugprone-argument-comment.CommentUserDefinedLiterals + value: '0' + - key: cert-str34-c.DiagnoseSignedUnsignedCharComparisons + value: 'false' + - key: bugprone-narrowing-conversions.WarnWithinTemplateInstantiation + value: 'false' + - key: cert-err33-c.CheckedFunctions + value: '::aligned_alloc;::asctime_s;::at_quick_exit;::atexit;::bsearch;::bsearch_s;::btowc;::c16rtomb;::c32rtomb;::calloc;::clock;::cnd_broadcast;::cnd_init;::cnd_signal;::cnd_timedwait;::cnd_wait;::ctime_s;::fclose;::fflush;::fgetc;::fgetpos;::fgets;::fgetwc;::fopen;::fopen_s;::fprintf;::fprintf_s;::fputc;::fputs;::fputwc;::fputws;::fread;::freopen;::freopen_s;::fscanf;::fscanf_s;::fseek;::fsetpos;::ftell;::fwprintf;::fwprintf_s;::fwrite;::fwscanf;::fwscanf_s;::getc;::getchar;::getenv;::getenv_s;::gets_s;::getwc;::getwchar;::gmtime;::gmtime_s;::localtime;::localtime_s;::malloc;::mbrtoc16;::mbrtoc32;::mbsrtowcs;::mbsrtowcs_s;::mbstowcs;::mbstowcs_s;::memchr;::mktime;::mtx_init;::mtx_lock;::mtx_timedlock;::mtx_trylock;::mtx_unlock;::printf_s;::putc;::putwc;::raise;::realloc;::remove;::rename;::scanf;::scanf_s;::setlocale;::setvbuf;::signal;::snprintf;::snprintf_s;::sprintf;::sprintf_s;::sscanf;::sscanf_s;::strchr;::strerror_s;::strftime;::strpbrk;::strrchr;::strstr;::strtod;::strtof;::strtoimax;::strtok;::strtok_s;::strtol;::strtold;::strtoll;::strtoul;::strtoull;::strtoumax;::strxfrm;::swprintf;::swprintf_s;::swscanf;::swscanf_s;::thrd_create;::thrd_detach;::thrd_join;::thrd_sleep;::time;::timespec_get;::tmpfile;::tmpfile_s;::tmpnam;::tmpnam_s;::tss_create;::tss_get;::tss_set;::ungetc;::ungetwc;::vfprintf;::vfprintf_s;::vfscanf;::vfscanf_s;::vfwprintf;::vfwprintf_s;::vfwscanf;::vfwscanf_s;::vprintf_s;::vscanf;::vscanf_s;::vsnprintf;::vsnprintf_s;::vsprintf;::vsprintf_s;::vsscanf;::vsscanf_s;::vswprintf;::vswprintf_s;::vswscanf;::vswscanf_s;::vwprintf_s;::vwscanf;::vwscanf_s;::wcrtomb;::wcschr;::wcsftime;::wcspbrk;::wcsrchr;::wcsrtombs;::wcsrtombs_s;::wcsstr;::wcstod;::wcstof;::wcstoimax;::wcstok;::wcstok_s;::wcstol;::wcstold;::wcstoll;::wcstombs;::wcstombs_s;::wcstoul;::wcstoull;::wcstoumax;::wcsxfrm;::wctob;::wctrans;::wctype;::wmemchr;::wprintf_s;::wscanf;::wscanf_s;' + - key: bugprone-suspicious-string-compare.WarnOnLogicalNotComparison + value: 'false' + - key: google-readability-braces-around-statements.ShortStatementLines + value: '1' + - key: bugprone-reserved-identifier.AllowedIdentifiers + value: '' + - key: bugprone-signal-handler.AsyncSafeFunctionSet + value: POSIX + - key: bugprone-suspicious-string-compare.WarnOnImplicitComparison + value: 'true' + - key: bugprone-argument-comment.CommentNullPtrs + value: '0' + - key: bugprone-narrowing-conversions.WarnOnIntegerToFloatingPointNarrowingConversion + value: 'true' + - key: bugprone-argument-comment.StrictMode + value: '0' + - key: bugprone-misplaced-widening-cast.CheckImplicitCasts + value: 'false' + - key: bugprone-suspicious-missing-comma.RatioThreshold + value: '0.200000' + - key: modernize-loop-convert.MinConfidence + value: reasonable + - key: bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField + value: 'true' + - key: google-readability-namespace-comments.ShortNamespaceLines + value: '10' + - key: google-readability-namespace-comments.SpacesBeforeComments + value: '2' + - key: cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic + value: 'true' + - key: bugprone-argument-comment.IgnoreSingleArgument + value: '0' + - key: bugprone-suspicious-string-compare.StringCompareLikeFunctions + value: '' + - key: bugprone-narrowing-conversions.WarnOnEquivalentBitWidth + value: 'true' + - key: bugprone-sizeof-expression.WarnOnSizeOfIntegerExpression + value: 'false' + - key: bugprone-assert-side-effect.CheckFunctionCalls + value: 'false' + - key: bugprone-narrowing-conversions.IgnoreConversionFromTypes + value: '' + - key: bugprone-string-constructor.StringNames + value: '::std::basic_string;::std::basic_string_view' + - key: bugprone-assert-side-effect.AssertMacros + value: assert,NSAssert,NSCAssert + - key: bugprone-exception-escape.IgnoredExceptions + value: '' + - key: bugprone-signed-char-misuse.DiagnoseSignedUnsignedCharComparisons + value: 'true' + - key: llvm-qualified-auto.AddConstToQualified + value: 'false' + - key: bugprone-narrowing-conversions.WarnOnIntegerNarrowingConversion + value: 'true' + - key: modernize-loop-convert.NamingStyle + value: CamelCase + - key: bugprone-suspicious-include.ImplementationFileExtensions + value: 'c;cc;cpp;cxx' + - key: bugprone-suspicious-missing-comma.SizeThreshold + value: '5' + - key: bugprone-suspicious-include.HeaderFileExtensions + value: ';h;hh;hpp;hxx' + - key: google-readability-function-size.StatementThreshold + value: '800' + - key: llvm-else-after-return.WarnOnConditionVariables + value: 'false' + - key: bugprone-argument-comment.CommentCharacterLiterals + value: '0' + - key: bugprone-argument-comment.CommentIntegerLiterals + value: '0' + - key: bugprone-stringview-nullptr.IncludeStyle + value: llvm + - key: bugprone-sizeof-expression.WarnOnSizeOfCompareToConstant + value: 'true' + - key: modernize-pass-by-value.IncludeStyle + value: llvm + - key: bugprone-reserved-identifier.AggressiveDependentMemberLookup + value: 'false' + - key: bugprone-sizeof-expression.WarnOnSizeOfThis + value: 'true' + - key: bugprone-string-constructor.WarnOnLargeLength + value: 'true' + - key: bugprone-too-small-loop-variable.MagnitudeBitsUpperLimit + value: '16' + - key: bugprone-argument-comment.CommentFloatLiterals + value: '0' + - key: modernize-use-nullptr.NullMacros + value: 'NULL' + - key: bugprone-dangling-handle.HandleClasses + value: 'std::basic_string_view;std::experimental::basic_string_view' + - key: bugprone-dynamic-static-initializers.HeaderFileExtensions + value: ';h;hh;hpp;hxx' + - key: bugprone-suspicious-enum-usage.StrictMode + value: 'false' + - key: bugprone-implicit-widening-of-multiplication-result.IncludeStyle + value: llvm + - key: bugprone-suspicious-missing-comma.MaxConcatenatedTokens + value: '5' + - key: bugprone-implicit-widening-of-multiplication-result.UseCXXHeadersInCppSources + value: 'true' + - key: llvm-else-after-return.WarnOnUnfixable + value: 'false' + - key: bugprone-not-null-terminated-result.WantToUseSafeFunctions + value: 'true' +... + diff --git a/CMakeLists.txt b/CMakeLists.txt index 50dce9417..931db7c40 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,6 +8,9 @@ endif() cmake_minimum_required(VERSION 3.15) +# generate a compilation database for static analysis by clang-tidy +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + # Set the project name to your project name project(RF24 C CXX) include(${CMAKE_CURRENT_LIST_DIR}/cmake/StandardProjectSettings.cmake) diff --git a/RF24.cpp b/RF24.cpp index da61a6514..f0f7a0879 100644 --- a/RF24.cpp +++ b/RF24.cpp @@ -551,7 +551,7 @@ uint8_t RF24::sprintf_address_register(char* out_buffer, uint8_t reg, uint8_t qt read_register(reg++, read_buffer, addr_width); uint8_t* bufptr = read_buffer + addr_width; while (--bufptr >= read_buffer) { - offset += sprintf_P(out_buffer + offset, PSTR("%02X"), *bufptr); + offset += sprintf_P(out_buffer + offset, PSTR("%02X"), *bufptr); // NOLINT(clang-analyzer-cplusplus.NewDelete) } } delete[] read_buffer; @@ -661,19 +661,6 @@ static const PROGMEM char* const rf24_pa_dbm_e_str_P[] = { rf24_pa_dbm_e_str_3, }; - #if defined(RF24_LINUX) -static const char rf24_csn_e_str_0[] = "CE0 (PI Hardware Driven)"; -static const char rf24_csn_e_str_1[] = "CE1 (PI Hardware Driven)"; -static const char rf24_csn_e_str_2[] = "CE2 (PI Hardware Driven)"; -static const char rf24_csn_e_str_3[] = "Custom GPIO Software Driven"; -static const char* const rf24_csn_e_str_P[] = { - rf24_csn_e_str_0, - rf24_csn_e_str_1, - rf24_csn_e_str_2, - rf24_csn_e_str_3, -}; - #endif // defined(RF24_LINUX) - static const PROGMEM char rf24_feature_e_str_on[] = "= Enabled"; static const PROGMEM char rf24_feature_e_str_allowed[] = "= Allowed"; static const PROGMEM char rf24_feature_e_str_open[] = " open "; @@ -760,7 +747,7 @@ void RF24::printPrettyDetails(void) (char*)(pgm_read_ptr(&rf24_pa_dbm_e_str_P[getPALevel()]))); printf_P(PSTR("RF Low Noise Amplifier\t" PRIPSTR "\r\n"), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[(read_register(RF_SETUP) & 1) * 1]))); + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast((read_register(RF_SETUP) & 1) * 1)]))); printf_P(PSTR("CRC Length\t\t" PRIPSTR "\r\n"), (char*)(pgm_read_ptr(&rf24_crclength_e_str_P[getCRCLength()]))); @@ -778,22 +765,22 @@ void RF24::printPrettyDetails(void) uint8_t features = read_register(FEATURE); printf_P(PSTR("Multicast\t\t" PRIPSTR "\r\n"), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(features & _BV(EN_DYN_ACK)) * 2]))); + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(static_cast(features & _BV(EN_DYN_ACK)) * 2)]))); printf_P(PSTR("Custom ACK Payload\t" PRIPSTR "\r\n"), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(features & _BV(EN_ACK_PAY)) * 1]))); + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(static_cast(features & _BV(EN_ACK_PAY)) * 1)]))); uint8_t dynPl = read_register(DYNPD); printf_P(PSTR("Dynamic Payloads\t" PRIPSTR "\r\n"), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[(dynPl && (features & _BV(EN_DPL))) * 1]))); + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast((dynPl && (features & _BV(EN_DPL))) * 1)]))); uint8_t autoAck = read_register(EN_AA); if (autoAck == 0x3F || autoAck == 0) { // all pipes have the same configuration about auto-ack feature printf_P(PSTR("Auto Acknowledgment\t" PRIPSTR "\r\n"), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(autoAck) * 1]))); + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(static_cast(autoAck) * 1)]))); } else { // representation per pipe @@ -859,21 +846,21 @@ uint16_t RF24::sprintfPrettyDetails(char* debugging_information) static_cast(getChannel() + 2400), (char*)(pgm_read_ptr(&rf24_datarate_e_str_P[getDataRate()])), (char*)(pgm_read_ptr(&rf24_pa_dbm_e_str_P[getPALevel()])), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[(read_register(RF_SETUP) & 1) * 1])), + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast((read_register(RF_SETUP) & 1) * 1)])), (char*)(pgm_read_ptr(&rf24_crclength_e_str_P[getCRCLength()])), ((read_register(SETUP_AW) & 3) + 2), getPayloadSize(), ((read_register(SETUP_RETR) >> ARD) * 250 + 250), (read_register(SETUP_RETR) & 0x0F), (read_register(OBSERVE_TX) >> 4), (read_register(OBSERVE_TX) & 0x0F), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(read_register(FEATURE) & _BV(EN_DYN_ACK)) * 2])), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(read_register(FEATURE) & _BV(EN_ACK_PAY)) * 1])), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[(read_register(DYNPD) && (read_register(FEATURE) & _BV(EN_DPL))) * 1]))); + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(static_cast(read_register(FEATURE) & _BV(EN_DYN_ACK)) * 2)])), + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(static_cast(read_register(FEATURE) & _BV(EN_ACK_PAY)) * 1)])), + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast((read_register(DYNPD) && (read_register(FEATURE) & _BV(EN_DPL))) * 1)]))); uint8_t autoAck = read_register(EN_AA); if (autoAck == 0x3F || autoAck == 0) { // all pipes have the same configuration about auto-ack feature offset += sprintf_P( debugging_information + offset, PSTR("" PRIPSTR ""), - (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(autoAck) * 1]))); + (char*)(pgm_read_ptr(&rf24_feature_e_str_P[static_cast(static_cast(autoAck) * 1)]))); } else { // representation per pipe @@ -1769,7 +1756,8 @@ bool RF24::writeAckPayload(uint8_t pipe, const void* buf, uint8_t len) bool RF24::isAckPayloadAvailable(void) { - return available(NULL); + uint8_t pipe = RF24_NO_FETCH_PIPE; + return available(&pipe); } /****************************************************************************/ diff --git a/RF24.h b/RF24.h index 5971826f5..f712750fe 100644 --- a/RF24.h +++ b/RF24.h @@ -12,8 +12,8 @@ * Class declaration for RF24 and helper enums */ -#ifndef __RF24_H__ -#define __RF24_H__ +#ifndef RF24_H_ +#define RF24_H_ #include "RF24_config.h" @@ -2410,4 +2410,4 @@ class RF24 * Use `ctrl+c` to quit at any time. */ -#endif // __RF24_H__ \ No newline at end of file +#endif // RF24_H_ diff --git a/RF24_config.h b/RF24_config.h index 7ed2aea95..8fbc7fb2e 100644 --- a/RF24_config.h +++ b/RF24_config.h @@ -16,8 +16,8 @@ version 2 as published by the Free Software Foundation. */ -#ifndef __RF24_CONFIG_H__ -#define __RF24_CONFIG_H__ +#ifndef RF24_CONFIG_H_ +#define RF24_CONFIG_H_ /*** USER DEFINES: ***/ #define FAILURE_HANDLING @@ -36,8 +36,8 @@ #endif /**********************/ -#define rf24_max(a, b) (a > b ? a : b) -#define rf24_min(a, b) (a < b ? a : b) +#define rf24_max(a, b) ((a) > (b) ? (a) : (b)) +#define rf24_min(a, b) ((a) < (b) ? (a) : (b)) /** @brief The default SPI speed (in Hz) */ #ifndef RF24_SPI_SPEED @@ -244,4 +244,4 @@ typedef uint16_t prog_uint16_t; #define RF24_SPI_TRANSACTIONS #endif // defined (SPI_HAS_TRANSACTION) && !defined (SPI_UART) && !defined (SOFTSPI) -#endif // __RF24_CONFIG_H__ +#endif // RF24_CONFIG_H_ diff --git a/examples_linux/CMakeLists.txt b/examples_linux/CMakeLists.txt index f75d8b771..b724753ba 100644 --- a/examples_linux/CMakeLists.txt +++ b/examples_linux/CMakeLists.txt @@ -11,6 +11,9 @@ set(EXAMPLES_LIST interruptConfigure ) +# generate a compilation database for static analysis by clang-tidy +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + project(RF24Examples CXX) add_compile_options(-Ofast -Wall) # passing the compiler a `-pthread` flag doesn't work here diff --git a/examples_pico/CMakeLists.txt b/examples_pico/CMakeLists.txt index fe1968489..19451ffe4 100644 --- a/examples_pico/CMakeLists.txt +++ b/examples_pico/CMakeLists.txt @@ -3,6 +3,9 @@ cmake_minimum_required(VERSION 3.12) # Pull in SDK (must be before project) include(../cmake/pico_sdk_import.cmake) +# generate a compilation database for static analysis by clang-tidy +set(CMAKE_EXPORT_COMPILE_COMMANDS ON) + project(pico_examples C CXX ASM) # Initialize the Pico SDK diff --git a/printf.h b/printf.h index 7c8d4b58e..df06c5299 100644 --- a/printf.h +++ b/printf.h @@ -14,8 +14,8 @@ * enables 'printf' */ -#ifndef __PRINTF_H__ -#define __PRINTF_H__ +#ifndef RF24_PRINTF_H_ +#define RF24_PRINTF_H_ #if defined(ARDUINO_ARCH_AVR) || defined(__ARDUINO_X86__) || defined(ARDUINO_ARCH_MEGAAVR) @@ -43,4 +43,4 @@ void printf_begin(void) #endif // defined(__ARDUINO_X86__) } -#endif // __PRINTF_H__ +#endif // RF24_PRINTF_H_ diff --git a/pyRF24/pyRF24.cpp b/pyRF24/pyRF24.cpp index d154a68cb..2c8dd5670 100644 --- a/pyRF24/pyRF24.cpp +++ b/pyRF24/pyRF24.cpp @@ -19,10 +19,9 @@ char* get_bytes_or_bytearray_str(bp::object buf) py_ba = buf.ptr(); if (PyByteArray_Check(py_ba)) return PyByteArray_AsString(py_ba); - else if (PyBytes_Check(py_ba)) + if (PyBytes_Check(py_ba)) return PyBytes_AsString(py_ba); - else - throw_ba_exception(); + throw_ba_exception(); return NULL; } @@ -33,10 +32,9 @@ int get_bytes_or_bytearray_ln(bp::object buf) py_ba = buf.ptr(); if (PyByteArray_Check(py_ba)) return PyByteArray_Size(py_ba); - else if (PyBytes_Check(py_ba)) + if (PyBytes_Check(py_ba)) return PyBytes_Size(py_ba); - else - throw_ba_exception(); + throw_ba_exception(); return 0; }