Reduce pollution in stddef.h and stdarg.h #1595
Open
Conversation
This file contains 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
These headers relied in __BEGIN_DECS/__END_DECLS being defined when sys/_types.h was included, but there's not a requirement that this be the case.
Use __attribute__((__aligned__(x))) and __alignof__(x) in places of local macros that ultimately wrap these. We don't actually support compilers that don't define these so there's no loss of generality. This mirrors Clang's stddef.h.
We no longer use it and it pollutes downstream headers.
Some headers need to support our non-portable visibility macros, but would be more portable if they provided less overall pollution (e.g., stddef.h provides __BEGIN_DECLS/__END_DECLS).
Switch to sys/_visible.h for visibility macros. Prefer __builtin_offsetof over __offset. sys/cdefs.h always defines __offsetof to __builtin_offsetof so just use the latter to remove a dependency on sys/cdefs.h. Realistically, we're never going to care about a compiler that doesn't supply this builtin. Add a somewhat questionable guard around the offsetof() definition because the compiler no longer thinks it the same as a number of other redundent definitions scattered around (e.g., in the openzfs codebase). It is infact the same and those defintions likely shouldn't exist at all.
Rely in sys/_visible for visibility macros and use __buitin_va_list instead of __va_list everywere we declare va_list.
While the type of va_list and implementation of va_*() psuedo functions varies (sometimes greatly) by architecture, it will always be defind by the compiler in a consistant way that does not require machine dependent handling. MFC after: 1 week
Switch to using sys/stdarg.h for va_list type and va_* builtins. Make an attempt to insert the include in a sensible place. Where style(9) was followed this is easy, where it was ignored, aim for the first block of sys/*.h headers and don't get too fussy or try to fix other style bugs.
|
Love visible splitting off. Many others call it "features" but I'm agnostic. Will look closely later. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This set of changes aims to decrease the number of things declared in stddef.h and stdarg.h.
The motivation is that software shouldn't be depending on incidental definitions of standard headers (someone recently found that some headers depended on __BEGIN_DECLS leaking in which caused an issue with stddef.h from clang.)
On the whole I've taken the view that it's OK to use bare
__builtin_functions when our cdefs.h abstraction is unconditional. I don't propose a sweeping migration to them, but IMO the hygiene benefits are worth it here.The more impactful changes will require an exp-run and we may not end up wanting them all, but I've put them up together for ease of commenting.