-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Backport 3.6: Include common header first #10526
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
base: mbedtls-3.6
Are you sure you want to change the base?
Backport 3.6: Include common header first #10526
Conversation
In library source files, the order of things should be: 1. Define macros that affect the behavior of system headers, such as `_POSIX_C_SOURCE` and `_GNU_SOURCE`. 2. Include the library's common header: `common.h`. It takes care of many things, including defining the library configuration, granting access to private fields in structures, and activating platform-specific hacks. 3. Possibly a few header inclusions and macro definitions. 4. Guard everything else by `#if defined(MBEDTLS_XXX_C)` or some such. Enforce this order in files that previously did things they shouldn't have before including `common.h`. To locate the potentially problematic files: ``` grep -m1 '^#' library/*.c | grep -v -F common.h ``` Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
davidhorstmann-arm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
ChangeLog.d/aesce-include.txt
Outdated
| @@ -0,0 +1,3 @@ | |||
| Bugfix | |||
| * Fix compilation errors in `aesce.c` in some Visual Studio builds. | |||
| Fixes #548. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TF-PSA-Crypto#548, maybe? In this repository it was very old different issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, thanks.
I amended the last commit, to also fix the changelog message (which would have caused GitHub to close #548 if it had been an open issue).
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
6c3e1a8 to
e45e036
Compare
Backport of Mbed-TLS/TF-PSA-Crypto#550. The structure is similar, but the commit "Include common.h before system headers" is a little different, both because the header name changed and because the set of affected files is slightly different.
PR checklist