-
Notifications
You must be signed in to change notification settings - Fork 7.2k
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
Pr/strict prototypes (IDFGH-757) #2937
Conversation
d46fbe2
to
a3957eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @M1cha, and sorry for taking so long to review it.
It seems it has picked up some merge conflicts, and might get a few more in the coming days since we will be moving some IDF files around. I can take it from here, rebase to the master branch, and resolve the conflicts — would this be okay for you? I'll keep the original commit authorship.
If not, please rebase these commits to master, removing the change to project.mk. I can then add CI related changes on top.
Thanks again.
@@ -355,7 +355,7 @@ CFLAGS := $(strip \ | |||
-std=gnu99 \ | |||
$(OPTIMIZATION_FLAGS) $(DEBUG_FLAGS) \ | |||
$(COMMON_FLAGS) \ | |||
$(COMMON_WARNING_FLAGS) -Wno-old-style-declaration \ | |||
$(COMMON_WARNING_FLAGS) -Wno-old-style-declaration -Wstrict-prototypes \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will probably not enable this for all projects — since we can't expect all our users to write -Wstrict-prototypes
-clean code (we aren't even doing it ourselves, yet!). However we can enable it in CI checks in the same way we enable -Wwrite-strings.
(Note this also needs to be done for the CMake build system)
It's fine if you rebase it and solve the conflicts, anything else would be a catch-up game with new commits for a big change like this. |
@M1cha Thanks for the contribution and sorry for the slow turnaround. The PR has been merged, thanks. |
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
Merges espressif/esp-idf#2937 * Original commit: espressif/esp-idf@c2764f6
While in C++
()
is the same as(void)
, in C it's not and can be quite dangerous.