Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dist/tools/cppcheck: fix all Cppcheck 1.82 errors #12764

Merged
merged 12 commits into from
Nov 21, 2019

Conversation

fjmolinas
Copy link
Contributor

Contribution description

cppcheck errors are making merging #12558 more difficult so this PR fixes this.

Testing procedure

  • Run locally: ./dist/tools/cppcheck/check.sh no errors should appear
  • Green Murdock

Please take a closer look at 1e671ac and c2f593f,

1e671ac I'm not sure if int sig = -1 is the best initial value for sig, although shouldn't really matter since its updated in the while loop..

c2f593f is because RTT_FREQUENCY doesn't have a default value so it results in a division by 0. Maybe there is a best way than duplicating the warning.

If preferred I can squash all commits fixing same errors.

Issues/PRs references

#12558 can benefit from this.

@fjmolinas fjmolinas added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 21, 2019
@fjmolinas fjmolinas mentioned this pull request Nov 21, 2019
@kaspar030
Copy link
Contributor

kaspar030 commented Nov 21, 2019

Sorry, I commented in the commit view, @fjmolinas can you find the spots or should I move the comments? edit I copied them over.

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, two little changes please.

sys/include/net/gnrc/gomach/gomach.h Outdated Show resolved Hide resolved
cpu/native/irq_cpu.c Outdated Show resolved Hide resolved
@kaspar030
Copy link
Contributor

BTW, with "Cppcheck 1.89" on my arch system, I get a lot more errors.

@kaspar030
Copy link
Contributor

Semicolon missing just before nib_table.c:55

@fjmolinas
Copy link
Contributor Author

Semicolon missing just before nib_table.c:55

Yep my bad, fixed.

@fjmolinas
Copy link
Contributor Author

BTW, with "Cppcheck 1.89" on my arch system, I get a lot more errors.

Argh annoying, I'll address this later today, need to switch to another subject now.

@fjmolinas fjmolinas changed the title dist/tools/cppcheck: fix all errors dist/tools/cppcheck: fix all Cppcheck 1.82 errors Nov 21, 2019
@fjmolinas
Copy link
Contributor Author

BTW, with "Cppcheck 1.89" on my arch system, I get a lot more errors.

Argh annoying, I'll address this later today, need to switch to another subject now.

Or deal with it later on, I think it would be good to get rid of #12558. I could open an issue and deal with it later.

@kaspar030
Copy link
Contributor

Or deal with it later on, I think it would be good to get rid of #12558. I could open an issue and deal with it later.

sure! small steps are good. please squash!

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

@fjmolinas
Copy link
Contributor Author

Opened #12771, I tackle it as soon as possible.

@fjmolinas
Copy link
Contributor Author

@kaspar030 can I hit the button?

@kaspar030 kaspar030 merged commit 4d84c4c into RIOT-OS:master Nov 21, 2019
@kaspar030
Copy link
Contributor

Thanks for dealing with this!

@fjmolinas fjmolinas deleted the pr_cpp_check_fix branch November 21, 2019 15:28
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants