Enable more build warnings for libc-bottom-half#755
Merged
alexcrichton merged 1 commit intoWebAssembly:mainfrom Mar 3, 2026
Merged
Enable more build warnings for libc-bottom-half#755alexcrichton merged 1 commit intoWebAssembly:mainfrom
libc-bottom-half#755alexcrichton merged 1 commit intoWebAssembly:mainfrom
Conversation
While wasi-libc is compiled with `-Wall -Wextra` it ended up disabling a lot of warnings as well to handle third-party code. This commit reorganizes things by continuing to compile everything with `-Wall -Wextra` but disabling lints is done exclusively for third-party code rather than for all code in wasi-libc. In practice this means that everything in `libc-bottom-half`, code written exclusively for this repository, now has more lints firing. This additionally applies to `test/src/*.c`. Much of this commit is handling these lints then and adjusting a few patterns in places where necessary. This commit additionally disables compiling with `-Werror` by default. CI still enables this by default, but it means that new lints/warnings won't be breaking for anyone else other than this repository's own CI which should have pinned versions of Clang being used in various places.
alexcrichton
commented
Mar 3, 2026
Comment on lines
+46
to
53
| filesystem_path_flags_t flags = 0; | ||
| if ((flag & AT_SYMLINK_FOLLOW) != 0) | ||
| flags |= FILESYSTEM_PATH_FLAGS_SYMLINK_FOLLOW; | ||
| bool ok = filesystem_method_descriptor_link_at(file_handle1, | ||
| 0, | ||
| flags, | ||
| &path1_wasi, | ||
| file_handle2, | ||
| &path2_wasi, |
Collaborator
Author
There was a problem hiding this comment.
This is an example bug caught by the warnings, turns out wasip2 wasn't looking at flag-the-parameter
alexcrichton
commented
Mar 3, 2026
Comment on lines
+924
to
+927
| if (optlen < sizeof(int)) { | ||
| errno = EINVAL; | ||
| return -1; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
This is also an example bug, optlen wasn't checked in this function anywhere else prior to this commit.
dicej
approved these changes
Mar 3, 2026
Collaborator
dicej
left a comment
dicej
left a comment
There was a problem hiding this comment.
Thanks so much for doing this!
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
2 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.
While wasi-libc is compiled with
-Wall -Wextrait ended up disabling a lot of warnings as well to handle third-party code. This commit reorganizes things by continuing to compile everything with-Wall -Wextrabut disabling lints is done exclusively for third-party code rather than for all code in wasi-libc. In practice this means that everything inlibc-bottom-half, code written exclusively for this repository, now has more lints firing. This additionally applies totest/src/*.c. Much of this commit is handling these lints then and adjusting a few patterns in places where necessary.This commit additionally disables compiling with
-Werrorby default. CI still enables this by default, but it means that new lints/warnings won't be breaking for anyone else other than this repository's own CI which should have pinned versions of Clang being used in various places.