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.

Sign up for GitHub

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

Jump to bottom

Improve compile warnings #458

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Improve compile warnings #458

wants to merge 10 commits into from

Conversation

ebassi
Copy link
Collaborator

@ebassi ebassi commented Oct 21, 2023

If we can, we should rejig the code to avoid declaring unused arguments and variables; if we can't, we can use G_GNUC_UNUSED to eliminate the warning.

@ebassi ebassi changed the title Get rid of compiler warnings Improve compile warnings Oct 21, 2023
@ebassi
Copy link
Collaborator Author

ebassi commented Oct 21, 2023

Actually, the compiler warnings situation is not nice. We're currently using warning_level=2 but:

  • we should really use warning_level=3
  • we should add more warnings to the Meson baseline

xdg-desktop-portal-gtk is a security boundary, and we should be more careful with the code we're shipping.

@ebassi ebassi marked this pull request as draft October 21, 2023 15:57
@TingPing
Copy link
Member

We're currently using warning_level=2 but we should really use warning_level=3

My personal opinion is -pedantic contains a lot of pointless annoyances and just including specific warnings has better results.

@ebassi
Copy link
Collaborator Author

ebassi commented Oct 21, 2023

I agree about pedantic, so even with warning_level=3 I'd drop it with additional compiler warnings; the other option is to add a bunch of warnings ourselves on top of warning_level=2.

@TingPing
Copy link
Member

TingPing commented Oct 21, 2023 •
edited
Loading

Just to clarify, warning_level=3 only adds -Wpedantic:

https://github.com/mesonbuild/meson/blob/ae7a9b0f4430a055ac419c6f1157cbd0e6eed45c/mesonbuild/compilers/c.py#L155-L158

@ebassi
Get rid of compiler warnings
47fc5de 
If we can, we should rejig the code to avoid declaring unused arguments
and variables; if we can't, we can use G_GNUC_UNUSED to eliminate the
warning.
@ebassi
Fix wrong prototype
ebcbf41
@ebassi
Fix wrong definition
fdc92db 
Missing `void`. This isn't C++.
@ebassi
Add missing prototypes
1cc2bb1
@ebassi
Use the appropriate format string
44e5b8c 
The title of the access dialog should not be used as the format string,
otherwise we open ourselves to specially crafted payloads making a mess.
@ebassi
Respect constness
60d4dc2
@ebassi
Add default condition
886c100 
Ensure we don't accidentally forget a state.
@ebassi
Mark internal functions as static
c6d36b5
@ebassi
Avoid shadowing variables with arguments
1b35948 
Make it clear what we're operating on.
@ebassi
build: Add more warnings
e36d27d 
Outside of Meson's default warnings, we really need to step up our
coverage.
@ebassi ebassi marked this pull request as ready for review October 24, 2023 18:04
TingPing
TingPing approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@TingPing TingPing left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GeorgesStavracas GeorgesStavracas GeorgesStavracas approved these changes

@TingPing TingPing TingPing approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ebassi @TingPing @GeorgesStavracas