Skip to content

Cleaning up Clang --warnlevel=1 warnings (reported in #23) #1419

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
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Cleaning up Clang --warnlevel=1 warnings (reported in #23) #1419

wants to merge 3 commits into from
+4 −4

Conversation

@onlyspxctre
Copy link

@onlyspxctre onlyspxctre commented Nov 17, 2025

#1406

Some of the warnings that were brought up in #23 had already been resolved, but these were the residual warnings I had encountered and fixed when compiling with Clang 19.

Rebased from master. First time contributor so I'm not really sure exactly how y'all want this, hopefully this is fine.

@onlyspxctre onlyspxctre mentioned this pull request Nov 17, 2025
Closed
stefan11111
stefan11111 reviewed Nov 17, 2025
View reviewed changes
test/misc.c

client->req_len = req.length = 1;
REQUEST_FIXED_SIZE(req, SIZE_MAX);
REQUEST_FIXED_SIZE(req, UINT_MAX);
Copy link
Contributor

@stefan11111 stefan11111 Nov 17, 2025

Choose a reason for hiding this comment

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

@metux is this right?
Doesn't this test what happens on overflow?

Copy link
Contributor

@metux metux Nov 17, 2025

Choose a reason for hiding this comment

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

indeed looks so.

@metux
Copy link
Contributor

metux commented Nov 17, 2025

#1406

Some of the warnings that were brought up in #23 had already been resolved, but these were the residual warnings I had encountered and fixed when compiling with Clang 19.

Rebased from master. First time contributor so I'm not really sure exactly how y'all want this, hopefully this is fine.

There's no need to create new PRs after rebase - you can use forced push (git push +)

@onlyspxctre
Copy link
Author

onlyspxctre commented Nov 17, 2025

#1406

Some of the warnings that were brought up in #23 had already been resolved, but these were the residual warnings I had encountered and fixed when compiling with Clang 19.

Rebased from master. First time contributor so I'm not really sure exactly how y'all want this, hopefully this is fine.

There's no need to create new PRs after rebase - you can use forced push (git push +)

Closing the old PR was not intentional; I renamed the branch without realizing that it would close the pull request.

@metux
Copy link
Contributor

metux commented Nov 18, 2025

#1406

Some of the warnings that were brought up in #23 had already been resolved, but these were the residual warnings I had encountered and fixed when compiling with Clang 19.

Rebased from master. First time contributor so I'm not really sure exactly how y'all want this, hopefully this is fine.

There's no need to create new PRs after rebase - you can use forced push (git push +)

Closing the old PR was not intentional; I renamed the branch without realizing that it would close the pull request.

By "renaming" you mean pushing as a new one and deleting the old one ?
Yes that closes PRs.

@onlyspxctre
Copy link
Author

onlyspxctre commented Nov 19, 2025

#1406

Some of the warnings that were brought up in #23 had already been resolved, but these were the residual warnings I had encountered and fixed when compiling with Clang 19.

Rebased from master. First time contributor so I'm not really sure exactly how y'all want this, hopefully this is fine.

There's no need to create new PRs after rebase - you can use forced push (git push +)

Closing the old PR was not intentional; I renamed the branch without realizing that it would close the pull request.

By "renaming" you mean pushing as a new one and deleting the old one ? Yes that closes PRs.

Yeah. It was accidental. Everything should be good now

@onlyspxctre
test: xi2: fix test_XIWarpPointer
e18bc1a 
Bit-shifting a negative integer leads to undefined behavior. Fixed by
negating the entire bitshift operation, rather than bitshifting a
negative value.

This fixes one of the Clang warnings as mentioned in X11Libre#23.

Signed-off-by: Spxctre <[email protected]>
@onlyspxctre
test: REQUEST_FIXED_SIZE expression is always true comparing `unsig…
cd6070c 
…ned int` to `SIZE_MAX`

`SIZE_MAX` is out of the range of `client->req_len` (unsigned int).
Compare `client->req_len` to `UINT_MAX` instead.

This fixes one of the Clang warnings as mentioned in X11Libre#23.

Signed-off-by: Spxctre <[email protected]>
@onlyspxctre
hw/xfree86/os-support: explicitly define SYSCALL macro with empty w…
adbcd75 
…hile body

The `SYSCALL` macro can seldom generate a warning in Clang when used
immediately before another macro (e.g. `DebugF`). We can explicitly
define the empty body to suppress these warnings.

Signed-off-by: Spxctre <[email protected]>
@metux metux requested a review from a team November 22, 2025 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@metux metux metux left review comments

@stefan11111 stefan11111 stefan11111 left review comments

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

@onlyspxctre @metux @stefan11111