Skip to content

cacertinmem.c example only loads one certificate - #3421

Closed
DaVieS007 wants to merge 3 commits into
curl:masterfrom
DaVieS007:master
Closed

DaVieS007 wants to merge 3 commits into
curl:masterfrom
DaVieS007:master

Conversation

@DaVieS007

Copy link
Copy Markdown
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

bagder commented Jan 1, 2019

Copy link
Copy Markdown
Member

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
Copy Markdown
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

bagder commented Jan 3, 2019

Copy link
Copy Markdown
Member

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

bagder commented Jan 3, 2019

Copy link
Copy Markdown
Member

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

@bagder

bagder commented Feb 1, 2019

Copy link
Copy Markdown
Member

@DaVieS007 any plans on adjusting this as suggested above?

@DaVieS007

Copy link
Copy Markdown
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

bagder commented Feb 27, 2019

Copy link
Copy Markdown
Member

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.

2 participants