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

meson.build: raise used C standard to C11 #365

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

ziyao233
Copy link
Contributor

Some C11 features have been used, but meson.build still specifies c_std=c99, leading to errors on Clang compiler.

Fixes: #364

Some C11 features have been used, but meson.build still specifies
c_std=c99, leading to errors on Clang compiler.

Fixes: containers#364
Signed-off-by: Yao Zi <[email protected]>
@cgwalters
Copy link
Contributor

Thanks for this! Actually testing with a somewhat recent clang (clang version 18.1.8) here I get compilation failures.

I went ahead and pushed a few additional commits that add a CI check with clang and also fix those errors.

@cgwalters
Copy link
Contributor

Also same question as the other PR re licensing: #344
Are you OK with those license terms?

@ziyao233
Copy link
Contributor Author

Also same question as the other PR re licensing: #344
Are you OK with those license terms?

It's okay to me :)

@cgwalters
Copy link
Contributor

I pushed some other changes here so I probably shouldn't merge without a 3rd reviewer.

@cgwalters
Copy link
Contributor

Especially the ioctl changes need careful review, I only sanity checked this. Hmm I may just mark as draft until we've done that.

@ziyao233 what version of clang are you using? It must be an older version that doesn't warn about some gnu-specific stuff?

Ohh or maybe the conceptual problem is those are just warnings but in CI we use -Werror, and so my attempt to add CI coverage clashed with that. Let me see...perhaps should just take your change as is and I'll split my patches to a separate PR.

@ziyao233
Copy link
Contributor Author

Especially the ioctl changes need careful review, I only sanity checked this. Hmm I may just mark as draft until we've done that.

@ziyao233 what version of clang are you using? It must be an older version that doesn't warn about some gnu-specific stuff?

Exactly 18.1.8 and it does throw some warnings, but I didnt come up with a graceful solution to some of them, especially fsverity related things. I decided to split warning fixes into another PR (they are just causing warnings but not error).

Ohh or maybe the conceptual problem is those are just warnings

Yep

but in CI we use -Werror, and so my attempt to add CI coverage clashed with that. Let me see...perhaps should just take your change as is and I'll split my patches to a separate PR.

seems a good idea to me

@cgwalters cgwalters merged commit caed86f into containers:main Sep 27, 2024
27 checks passed
@cgwalters cgwalters mentioned this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build failure with Clang 18
2 participants