Feature/fixing clang gnu compiler warnings

[phelter]

Fixing majority of the clang compiler warnings.

Description

Fixed majority of clang and gnu compiler warnings. Primary ones are:

• misuse of doxygen single line comments
• replaced wrong type usage for Interrupt return and clear interrupt macros.

List of compiler warnings goes from:

target_compile_options( freertos_kernel
  PRIVATE
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-cast-align>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-covered-switch-default>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-documentation>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-extra-semi-stmt>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-int-to-pointer-cast>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-noreturn>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-pointer-to-int-cast>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-shorten-64-to-32>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wno-unused-variable>
)
# For linux port
target_compile_options( freertos_kernel_port
  PRIVATE
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-disabled-macro-expansion>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-documentation>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-noreturn>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-pointer-to-int-cast>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-reserved-macro-identifier>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
    $<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-type-limits>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
)

To:

target_compile_options( freertos_kernel
  PRIVATE
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-cast-align>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-covered-switch-default>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-missing-variable-declarations>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
)
target_compile_options( freertos_kernel_port
  PRIVATE
     $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-disabled-macro-expansion>
     $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-padded>
     $<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-reserved-macro-identifier>
)

Test Steps

Compile with:

add_compile_options(
    ### C Based Language and ID.
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wall>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wextra>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wpedantic>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Werror>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Weverything>
)

Related Issue

Tried to have this list in the CMakeLists.txt originally in PR #571, but was overruled, so reducing list for future checks.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[RichardBarry]

Looks like this is in hand already, some notes all the same:

1. We can't have any compiler specific code in common files, only in the port layer. Compiler specific code includes preprocessor macros that test which compiler is in use.
2. Make sure updates comment block characters till work with Doxygen.

[phelter]

Looks like this is in hang already, some notes all the same:

1. We can't have any compiler specific code in common files, only in the port layer.  Compiler specific code includes preprocessor macros that test which compiler is in use.

Fixed with latest commit.

2. Make sure updates comment block characters till work with Doxygen.

This can be done by running a clang compile with the following compile options:

add_compile_options(
    ### C Based Language and ID.
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wall>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wextra>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wpedantic>
    $<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Werror>
    $<$<COMPILE_LANG_AND_ID:C,Clang>:-Weverything>
)

the -Wdocumentation-* checks validity of doxygen information - i.e. invalid or missing params, and wrong usage of doxygen. You can also run Doxygen on the entire suite of C files using CMake and have warnings as errors defined when attempting to create the documentation as one of the build steps.

[aggarg]

Some checks are failing likely because of the syntax error in timers.c. Please look into the failed checks.

Thank you again for your contribution.

[phelter]

@RichardBarry and @aggarg - the changes requested have all been performed and the unit tests now pass. I'd suggest adding a build with clang and would also suggest using clang-tidy on this code base as well.

[phelter]

Ping @RichardBarry and @aggarg . Would like to no longer maintain the fork that I have in use, so please approve at your earliest convenience.

[codecov]

Codecov Report

Base: 94.30% // Head: 94.34% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (75f2405) compared to base (5d05601).
Patch coverage: 90.90% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #620      +/-   ##
==========================================
+ Coverage   94.30%   94.34%   +0.03%     
==========================================
  Files           6        6              
  Lines        2370     2368       -2     
  Branches      579      578       -1     
==========================================
- Hits         2235     2234       -1     
  Misses         85       85              
+ Partials       50       49       -1     

Flag	Coverage Δ
unittests	94.34% <90.90%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
timers.c	100.00% <ø> (ø)
stream_buffer.c	97.16% <83.33%> (+0.29%)	⬆️
queue.c	93.95% <91.66%> (ø)
tasks.c	94.69% <92.30%> (ø)
event_groups.c	81.38% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Note code coverage for this line is not expected. There is no change except to remove the static from the function definition because it is already defined in the function declaration. Please waive changed CodeCov.

Note only changing the type here because the original had an incorrect type. These are defined by the port types - portBaseType and not by the kernel types - UBaseType_t. Please waive changed CodeCov .

Ping @RichardBarry and @aggarg . Would like to no longer maintain the fork that I have in use, so please approve at your earliest convenience.

Apologies for the delay. Will get it done today.

Thank you for your contribution.

@phelter Why do you need C99? We want to support C89. Did you notice anything C99 specific in the kernel?

@aggarg - Yes- there were several cases of: C90 ISO violations of declaration after statement. See: https://clang.llvm.org/docs/DiagnosticsReference.html#wdeclaration-after-statement These may have since been updated.

These can be detected using clang-14 compiles by setting the compile options in the description of this PR.

Feel free to change back to C90 if/when those are updated. I am unaware of any others.

Thank you for the clarification. I did build the kernel with -Wdeclaration-after-statement and I see no warning from kernel source. I will remove the following 2 lines which were added in this PR -

set(CMAKE_C_STANDARD 99)
set(CMAKE_C_STANDARD_REQUIRED ON)

Here is the PR - #633