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

mkerr: remove legacy guards from generated error headers #11541

Closed
wants to merge 1 commit into from

Conversation

mspncp
Copy link
Contributor

@mspncp mspncp commented Apr 13, 2020

In pull request #9333, legacy guards were added to the generated
error headers, but the mkerr.pl script was not adjusted accordingly.
So the legacy guards were removed by subsequent make update calls.

Fixing the mkerr.pl script properly was disproportionately complicated
by the fact that adding legacy guards only made sense for files which
already existed in version 1.1.1. To keep things simple, it was decided
to drop the legacy guards from the generated headers entirely.

Fixes #10569

In pull request openssl#9333, legacy guards were added to the generated
error headers, but the mkerr.pl script was not adjusted accordingly.
So the legacy guards were removed by subsequent `make update` calls.

Fixing the mkerr.pl script properly was disproportionately complicated
by the fact that adding legacy guards only made sense for files which
already existed in version 1.1.1. To keep things simple, it was decided
to drop the legacy guards from the generated headers entirely.

Fixes openssl#10569
@@ -450,6 +450,7 @@ sub help

#ifndef OPENSSL_${lib}ERR_H
# define OPENSSL_${lib}ERR_H
# pragma once
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(Note: Adding the missing pragma is the only remaining fix for mkerr.pl)

@mspncp mspncp added approval: review pending This pull request needs review by a committer branch: master Merge to master branch labels Apr 13, 2020
@levitte levitte 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 Apr 14, 2020
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Apr 15, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Apr 15, 2020
In pull request #9333, legacy guards were added to the generated
error headers, but the mkerr.pl script was not adjusted accordingly.
So the legacy guards were removed by subsequent `make update` calls.

Fixing the mkerr.pl script properly was disproportionately complicated
by the fact that adding legacy guards only made sense for files which
already existed in version 1.1.1. To keep things simple, it was decided
to drop the legacy guards from the generated headers entirely.

Fixes #10569

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #11541)
@mspncp
Copy link
Contributor Author

mspncp commented Apr 15, 2020

Merged to master as 9bf475f, thanks!

@mspncp mspncp closed this Apr 15, 2020
@mspncp mspncp deleted the pr-remove-mkerr-legacy-guards branch April 15, 2020 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

util/mkerr.pl does not add legacy include guards
3 participants