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.

Sign up for GitHub

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

Jump to bottom

Correct and simplify use of ERR_clear_error() etc. for loading DSO libs #13045

Closed
wants to merge 2 commits into from
Closed

Correct and simplify use of ERR_clear_error() etc. for loading DSO libs #13045

wants to merge 2 commits into from

Conversation

DDvO
Copy link
Contributor

@DDvO DDvO commented Sep 30, 2020 •
edited

After app startup, depending on config file entries regarding engines, sometimes spurious error messages show up such as

C0A7DE58037F0000:error::DSO support routines:(unknown function):could not load the shared library:crypto/dso/dso_lib.c:162:
C0A7DE58037F0000:error::configuration file routines:(unknown function):error loading dso:crypto/conf/conf_mod.c:285:module=engines, path=engines
C0A7DE58037F0000:error::configuration file routines:(unknown function):unknown module name:crypto/conf/conf_mod.c:225:module=engines

This is cleaned up by this PR, also correcting and simplifying related types of error queue resets.

levitte
levitte reviewed Sep 30, 2020
View reviewed changes
crypto/conf/conf_mod.c Outdated Show resolved Hide resolved
@levitte
Copy link
Member

levitte commented Sep 30, 2020

Apart from one comment, this looks like very good work, well done!

@slontis
Copy link
Member

slontis commented Sep 30, 2020

Please start setting up the labels on your PR's

@levitte levitte added approval: review pending This pull request needs review by a committer branch: master Merge to master branch labels Oct 1, 2020
@levitte levitte added this to the 3.0.0 milestone Oct 1, 2020
@DDvO DDvO force-pushed the cleanup_Err_clear_error_DSO branch from 9b41606 to 66259a9 Compare October 1, 2020 14:09
@DDvO DDvO added approval: otc review pending This pull request needs review by an OTC member and removed approval: review pending This pull request needs review by a committer labels Oct 1, 2020
@DDvO
Copy link
Contributor Author

DDvO commented Oct 1, 2020

Please start setting up the labels on your PR's

I guess you meant the label "approval: otc review pending"?
I tend to add it when the CI has passed, but I can to also right away.

@DDvO
Copy link
Contributor Author

DDvO commented Oct 6, 2020

Can some OTC member please approve this?
@levitte just mentioned a minor improvement to be one, which I did.

levitte
levitte approved these changes Oct 6, 2020
View reviewed changes
Copy link
Member

@levitte levitte left a comment

Choose a reason for hiding this comment

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

Okie

@levitte levitte added approval: done This pull request has the required number of approvals and removed approval: otc review pending This pull request needs review by an OTC member labels Oct 6, 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 Oct 7, 2020
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Oct 8, 2020
@DDvO
Correct and simplify use of ERR_clear_error() etc. for loading DSO libs
55c6147 
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #13045)
@DDvO
Copy link
Contributor Author

DDvO commented Oct 8, 2020

Merged - thanks @levitte

@DDvO DDvO closed this Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@levitte levitte levitte approved these changes

Assignees
No one assigned
Labels
approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Merge to master branch
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@DDvO @levitte @slontis @openssl-machine