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

Don't attempt to include system headers when not required #7813

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

SparkiDev
Copy link
Contributor

@SparkiDev SparkiDev commented Jul 30, 2024

Description

Some builds don't require system headers: no filesystem and single threaded.

Testing

./configure '--disable-shared' '--disable-filesystem' '--enable-singlethreaded'

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@SparkiDev SparkiDev assigned SparkiDev and wolfSSL-Bot and unassigned SparkiDev Jul 30, 2024
@SparkiDev SparkiDev requested a review from wolfSSL-Bot July 31, 2024 22:33
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

I believe this would be solved with SINGLE_THREADED and NO_FILESYSTEM. Which headers were being included? What use case is this solving? Is it tied to a support ticket?

For example I still see:

types.h:

#else /* fallback to architecture size_t for pointer size */
    #include <stddef.h> /* included for getting size_t type */
    typedef size_t wc_ptr_t;

types.h:

#else
                #include <stdio.h>
                #define XSNPRINTF snprintf

Also had to add -DWOLFSSL_NO_ATOMICS to get rid of atomics #include <stdatomic.h>.

Just to name a few. I was able to use -save-temps=obj to see the headers getting pulled in.

@SparkiDev
Copy link
Contributor Author

The customer didn't open a PR, it is part of the Dilithium work.

My concern was that we include other system headers in the alternate branches that aren't related to threading and filesystem. Excluding all system headers is what they want to do.

Would you rahter I changed the condition to single threaded and no filesyetem?

@dgarske
Copy link
Contributor

dgarske commented Aug 2, 2024

The customer didn't open a PR, it is part of the Dilithium work.

My concern was that we include other system headers in the alternate branches that aren't related to threading and filesystem. Excluding all system headers is what they want to do.

Would you rahter I changed the condition to single threaded and no filesyetem?

The else case at the bottom already guards on single_threaded and no_filesystem. Are you seeing something besides the else case like USE_WINDOWS_API? If so I think you should just change it to gate this section on those two macros instead of introducing a new macro.

@SparkiDev
Copy link
Contributor Author

Changed code to not try to include header files when single threaded and no filesystem.

@SparkiDev SparkiDev changed the title Compile flag to not include system headers Don't attempt to include system headers when not required Aug 5, 2024
@dgarske dgarske self-assigned this Aug 5, 2024
Some builds don't require system headers: no filesystem and single
threaded.
@dgarske dgarske force-pushed the no_system_headers branch from b4aee7c to 0e0c363 Compare August 5, 2024 17:49
@dgarske dgarske merged commit 039853c into wolfSSL:master Aug 5, 2024
122 checks passed
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.

3 participants