Skip to content

Comments

CORE & PROV: make export of key data leaner through callback, take 2#10414

Closed
levitte wants to merge 2 commits intoopenssl:masterfrom
levitte:lean-export-key-data-2
Closed

CORE & PROV: make export of key data leaner through callback, take 2#10414
levitte wants to merge 2 commits intoopenssl:masterfrom
levitte:lean-export-key-data-2

Conversation

@levitte
Copy link
Member

@levitte levitte commented Nov 11, 2019

Exporting data from a provider owned domainparams or key is quite an
ordeal, with having to figure out what parameter keys an
implementation supports, call the export function a first time to find
out how large each parameter buffer must be, allocate the necessary
space for it, and call the export function again.

So how about letting the export function build up the key data params
and call back with that? This change implements exactly such a
mechanism.


This is an alternative to #10391

@levitte levitte added branch: master Applies to master branch approval: review pending This pull request needs review by a committer labels Nov 11, 2019
@levitte levitte added this to the 3.0.0 milestone Nov 11, 2019
@mattcaswell
Copy link
Member

Travis failures are relevant

@levitte
Copy link
Member Author

levitte commented Nov 12, 2019

Darnit

@levitte
Copy link
Member Author

levitte commented Nov 12, 2019

Oh wait, that's because #10412 isn't merged yet, and I haven't included it in this PR.
Comments on the code and the idea would still be appreciated.

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

This looks a lot neater. I just have one comment.

Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Approved subject to a clean CI run once #10412 is merged.

@mattcaswell mattcaswell 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 Nov 12, 2019
Exporting data from a provider owned domainparams or key is quite an
ordeal, with having to figure out what parameter keys an
implementation supports, call the export function a first time to find
out how large each parameter buffer must be, allocate the necessary
space for it, and call the export function again.

So how about letting the export function build up the key data params
and call back with that?  This change implements exactly such a
mechanism.
@levitte levitte force-pushed the lean-export-key-data-2 branch from 0039a75 to 2a7224b Compare November 14, 2019 07:26
@levitte
Copy link
Member Author

levitte commented Nov 14, 2019

#10412 now being merged, I've rebased this

@levitte levitte changed the title [Pending on #10412] CORE & PROV: make export of key data leaner through callback, take 2 CORE & PROV: make export of key data leaner through callback, take 2 Nov 14, 2019
@levitte
Copy link
Member Author

levitte commented Nov 14, 2019

CIs are happy, apart from the Appveyor things that aren't relevant here

openssl-machine pushed a commit that referenced this pull request Nov 14, 2019
Exporting data from a provider owned domainparams or key is quite an
ordeal, with having to figure out what parameter keys an
implementation supports, call the export function a first time to find
out how large each parameter buffer must be, allocate the necessary
space for it, and call the export function again.

So how about letting the export function build up the key data params
and call back with that?  This change implements exactly such a
mechanism.

Reviewed-by: Matt Caswell <[email protected]>
(Merged from #10414)
@levitte
Copy link
Member Author

levitte commented Nov 14, 2019

Merged.

1640d48 CORE & PROV: make export of key data leaner through callback

@levitte levitte closed this Nov 14, 2019
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: master Applies to master branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants