FNA-XNA / FAudio

FAudio - Accuracy-focused XAudio reimplementation for open platforms
https://fna-xna.github.io/
Other
544 stars 73 forks source link

clang-tidy warnings #79

Closed NeroBurner closed 5 years ago

NeroBurner commented 5 years ago

the variable i is defined as uint16_t and compared in a for loop for the break condition i>=0 which is always true. i just wraps around and maybe can cause a segmentation fault

https://github.com/FNA-XNA/FAudio/blob/master/src/FACT_internal.c#L90 https://github.com/FNA-XNA/FAudio/blob/master/src/FACT_internal.c#L114

flibitijibibo commented 5 years ago

The break condition is definitely crappy, but at the same time the code should always break in the middle. If nothing else uses i we can change to an int32_t and that'll make it safer, but maybe we can make this loop nicer in general.

pchome commented 5 years ago

FYI: you can detect more of them by using e.g. clang-tidy.

clang-tidy can utilize compile_commands.json generated for Ninja, so I'm usually using it like this:

meson --prefix /tmp/faud /tmp/faud64
cd /tmp/faud64
/usr/lib/llvm/7/share/clang/run-clang-tidy.py -header-filter='.*' \
-extra-arg="-DHAVE_FFMPEG" -extra-arg="-isystem/usr/include/SDL2" \
-checks='-clang-diagnostic-unused-command-line-argument,-clang-diagnostic-implicit-function-declaration' -quiet

Output: FAudio-clang-tidy-output.txt

Checks and descriptions: https://clang.llvm.org/extra/clang-tidy/checks/list.html

Example2: all checks disabled, except bugprone-* ones, and "auto-fix" (if possible) allowed

$ clang-tidy -header-filter=.* -checks=-*,bugprone-* -extra-arg=-isystem/usr/include/SDL2 -p=/tmp/faud64 -quiet -fix /tmp/FAudio/src/XNA_Song.c
flibitijibibo commented 5 years ago

Some commits to deal with these warnings:

https://github.com/FNA-XNA/FAudio/commit/5609c6b21014b67a8349ed32cfeca04f901702e1 https://github.com/FNA-XNA/FAudio/commit/ab45f64d556675a9e54021cb1988bf0ad2163f16 https://github.com/FNA-XNA/FAudio/commit/87816417ecf57be724ebc16a8fb07d6b48314ca0 https://github.com/FNA-XNA/FAudio/commit/87bfe65c09339a85bac0c9036b7eeb50503791cb

Probably won't modify any of the stb_vorbis stuff unless Sean is interested in those changes.

NeroBurner commented 5 years ago

clang tidy can also be started directly through cmake, just tried it with the following commands

cmake -H. -B_build -DCMAKE_C_CLANG_TIDY=clang-tidy -DCMAKE_CXX_CLANG_TIDY=clang-tidy  -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
cmake --build _build

this builds libFAudio.so with clang and outputs clang-tidy warnings while building.

flibitijibibo commented 5 years ago

This all works for me - ran the tool locally (F28 version) and the build/output seems good. Glad that I can start using this stuff again... let's make sure this keeps working throughout the rest of the CMake work.