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

cacertinmem.c example only loads one certificate #3421

Closed
wants to merge 3 commits into from

Conversation

DaVieS007
Copy link
Contributor

Seems this example showing the only way to load CA file (curl-ca-bundle.crt) from memory now able to process the whole certificate-chain from memory.

@bagder
Copy link
Member

bagder commented Jan 1, 2019

This is an example. I don't see how it serves anyone better if we add 4000 lines of PEM data into it...

@DaVieS007
Copy link
Contributor Author

This is an example. I don't see how it serves anyone better if we add 4000 lines of PEM data into it...

Obviously not that 4000 lines are the subject, I inserted it to make it "work" with same method, maybe it should be load from file to demonstrate "memory" operation.

@bagder
Copy link
Member

bagder commented Jan 3, 2019

I think improving the example to work with more than one cert in memory is a good idea, but then I think having two in there is good enough for the example. Wouldn't you agree?

@bagder
Copy link
Member

bagder commented Jan 3, 2019

Additionally, also run make checksrc and you'll spot a few style nits to fix.

@bagder
Copy link
Member

bagder commented Feb 1, 2019

@DaVieS007 any plans on adjusting this as suggested above?

@DaVieS007
Copy link
Contributor Author

@bagder yes for sure after I find out what causing memory leak and the code above is affacted or not.

@bagder bagder closed this in 0f6c6ef Feb 27, 2019
@bagder
Copy link
Member

bagder commented Feb 27, 2019

Thanks @DaVieS007 for your work on this. I did some minor edits to the code before it landed.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants