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

Remove externally used include guards [1.1.1] #9365

Conversation

mspncp
Copy link
Contributor

@mspncp mspncp commented Jul 14, 2019

This is a backport of pull request #9364 to 1.1.1

The only commit which had some minor conflicts is the last commit

  • Remove HEADER_X509_H include detector from apps

The conflict was caused by reorganization of the internal app headers between 1.1.1 and master.

It has a fixup because of a compiler warning (as error) about _kbhit().

This include guard inside an object file comes as a surprise and
serves no purpose anymore. It seems like this object file was
included by crypto/threads/mttest.c at some time, but the include
directive was removed in commit bb8abd6.
The check is redundant, because <openssl/x509v3.h> is included.
The HEADER_X509_H check is redundant, because <openssl/x509.h>
is already included.
@mspncp mspncp added the branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch label Jul 14, 2019
@mspncp mspncp force-pushed the pr-remove-externally-used-include-guards-111 branch from 25ee615 to a7cc91d Compare July 14, 2019 13:17
Move _kbhit() from s_apps.h to apps.c
@mspncp mspncp force-pushed the pr-remove-externally-used-include-guards-111 branch from a7cc91d to 7cec6f7 Compare July 14, 2019 16:26
@mspncp mspncp added the approval: review pending This pull request needs review by a committer label Jul 17, 2019
Copy link
Member

@bernd-edlinger bernd-edlinger left a comment

Choose a reason for hiding this comment

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

Just a minor nit.

util/mkerr.pl Outdated Show resolved Hide resolved
@mspncp
Copy link
Contributor Author

mspncp commented Jul 24, 2019

ping @levitte?

@levitte
Copy link
Member

levitte commented Jul 24, 2019

Sorry I took some time. As you may have seen, I have quite a set of distractions 😉

@mspncp
Copy link
Contributor Author

mspncp commented Jul 24, 2019

Yes, I know that you are very busy currently. And I appreciate that you took the time to look.

@mspncp mspncp added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Jul 24, 2019
levitte pushed a commit that referenced this pull request Jul 24, 2019
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #9365)
levitte pushed a commit that referenced this pull request Jul 24, 2019
This include guard inside an object file comes as a surprise and
serves no purpose anymore. It seems like this object file was
included by crypto/threads/mttest.c at some time, but the include
directive was removed in commit bb8abd6.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #9365)
levitte pushed a commit that referenced this pull request Jul 24, 2019
The check is redundant, because <openssl/x509v3.h> is included.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #9365)
levitte pushed a commit that referenced this pull request Jul 24, 2019
The HEADER_X509_H check is redundant, because <openssl/x509.h>
is already included.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from #9365)
@mspncp
Copy link
Contributor Author

mspncp commented Jul 24, 2019

Merged to 1.1.1, thanks!

77cb243 Remove HEADER_X509_H include detector from apps
19b7b64 Remove OPENSSL_X509V3_H include detector from openssl/cms.h
ca06621 Remove HEADER_BSS_FILE_C module include guard
0904e31 Remove external HEADER_SYMHACKS_H include guard

@mspncp mspncp closed this Jul 24, 2019
@mspncp mspncp deleted the pr-remove-externally-used-include-guards-111 branch September 30, 2019 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: 1.1.1 Merge to OpenSSL_1_1_1-stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants