Big clean-up PR!#111
Merged
Merged
Conversation
clang-tidy uses compile_commands.json as a source of truth for compiler and linker options. However, files under src/ might be compiled twice: once for bpfilter and once for the unit tests. The unit tests build has specific flags that might generate erroneous clang-tidy warning (e.g. with mock_assert() and null pointer dereference). To solve this issue, jq is used to filter out every file from tests/unit and create a new compile_commands.json to use as a source for clang-tidy.
bf_logger_get_color() requires a bf_style enum argument, but it was passed a combination of bf_style arguments (e.g. BF_STYLE_BLUE | BF_STYLE_BOLD) which passes an invalid bf_style value. Change bf_logger_get_color() to receive a bf_color and a bf_style argument.
Set destructors for custom parser types to avoid memory leaks on YYABORT.
Create struct bf_ruleset to contain a parsed ruleset and use it as an argument to yyparse(). Define a macro bf_parse_err() to call yyerror() and YYABORT.
make lint was sitting unused as a clang-tidy is slow and an overwhelming amount of warning would be spitted out by clang. Only make checkstyle was used to validate the code's style. Now that clang-tidy's warning are fixed, a new target has been added: make check. This new target will run clang-tidy and clang-format (with --dry-run) for every source file. A custom CMake command has been defined for each source file to check, so clang-tidy can be run in parallel (instead of calling it once with all the source files as argument). On my host, with 9 threads, linting all the files in src/ takes ~4 seconds instead of ~12 seconds.
make fixstyle makes it easier to make clang-format fix the formatting warnings for files under src/. Use it with caution: ensure the fix doesn't modify the code's behavior.
Ubuntu's Doxygen version has different option than Fedora's version and the documentation generation will fail when it tries to generate the DOT representation of the project. Explicitly disable DOT to avoid this issue.
Simplify the user-facing logging macros and move the coloring and alignment logic into the common implementation.
As per the main CMakeLists.txt, NDEBUG is never defined, so there is no reason to gate bf_dbg() macros behind it. Also, the --verbose option will already filter those logs out.
Rename bf_err_code() (and similar functions) as bf_err_r(), this way the function name is shorter, and future addition (e.g. bf_err_v() for va_list) can follow the same pattern.
Introduce bf_err_v() (and similar functions for other log levels) to print a log message with a format and arguments from a va_list. This function is required to print error from yyerror().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix all existing
clang-tidywarnings (or update the configuration file if needed), somake checkcan runclang-formatandclang-tidyand be used from the CI. Main changes:jq(added as a dependency) to copycompile_commands.jsonand remove files undertests/, otherwiseclang-tidywould analyse the same file twice (once forsrc/and once fortests/unit/).BF_STYLE_GREEN | BF_STYLE_BOLD), creating invalid enum values.parser.y.bfcli's parser error management logic: wrap calls toyyerror()and log throughbpfilter's logger.make testtarget intotests/unit/CMakeLists.txt, with thetests_unittarget.make checkto run ClangFormat and ClangTidy. Add custom commands to runclang-tidyfor each source file, allowingmake checkto benefit from parallel builds.bf_err_v()(for other log levels too) to log from a format and ava_listof arguments.clang-tidywarnings, update.clang-tidyaccordingly.