Fix (and silence) a bunch of build-time diagnostics ("warnings")#1278
Merged
Fix (and silence) a bunch of build-time diagnostics ("warnings")#1278
Conversation
SebKuzminsky
requested changes
Oct 23, 2021
Collaborator
SebKuzminsky
left a comment
SebKuzminsky
left a comment
There was a problem hiding this comment.
Looks like a couple of copy/paste errors.
Collaborator
|
Aha. Then it is perhaps time to rebase the pull request on master and
see if we can get it merged before it is too late this time?
…--
Happy hacking
Petter Reinholdtsen
|
Collaborator
|
The *QUIETFLAGS Makefile variables seem scary, hiding coding errors that could cause security or stability issues. Perhaps we should make an effort to get completely rid of them? |
Collaborator
|
Note, I tried addressing one of the warnings in #1922 by completely rewriting the print methods to avoid statically allocated buffers. |
jepler
force-pushed
the
jepler/warnings-fixes
branch
2 times, most recently
from
7b58951 to
e53842d
Compare
Contributor
Author
|
I've heavily revised this PR and would appreciate re-review @SebKuzminsky @petterreinholdtsen |
Collaborator
petterreinholdtsen
left a comment
petterreinholdtsen
left a comment
There was a problem hiding this comment.
The code changes look good, more unsure about the build option changes, as some of them only hide the warnings, but do not solve the issues.
We deliberately use truncating formats and string operations, this is not something the compiler should warn us about.
These occur internally to boost::python headers.
4/5 based on a review comment by seb, 1 more caught by grep
.. so rename it. I don't know if anyone is using the rule, but if so they have to adjust their manual process This also fixes the build-time diagnostic ``` Makefile:865: warning: overriding recipe for target 'install-menu' Makefile:648: warning: ignoring old recipe for target 'install-menu' ```
.. it's ignored in the caller, but this should be the return value. It quiets a diagnostic about res being assigned but not used.
.. the manpage is still generated. This is done simply to get rid of the diagnostic message when it's built in-tree. I considered the alternative of providing a TOPDIR value during build, but as I would only be able to test it during a uspace build I decided not to do so. I'd be happy to see it done as a substitute, though, as otherwise the component is likely to bitrot.
.. it quiets some stringop-truncation diagnostics
jepler
force-pushed
the
jepler/warnings-fixes
branch
from
37f6eaf to
e489bc2
Compare
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
4 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.
No description provided.