Skip to content

Chore: Buffered_Computation can deal with generic containers#3294

Merged
reneme merged 4 commits intomasterfrom
chore/generic_interface_bufcomp
Mar 17, 2023
Merged

Chore: Buffered_Computation can deal with generic containers#3294
reneme merged 4 commits intomasterfrom
chore/generic_interface_bufcomp

Conversation

@reneme
Copy link
Copy Markdown
Collaborator

@reneme reneme commented Feb 16, 2023

This should be a transparent change for the most part but enables modern support for all sorts of containers without resorting to the C-style pointer APIs.

Allows passing all sorts of contiguous ranges to ::update() and ::process() as well as storing results of Buffered_Computation in user-defined contiguous containers.

The latter is most-useful for the strong types:

using MessageHash = Strong<std::vector<uint8_t>, struct MessageHash_>;

auto sha = HashFunction::create("SHA-256");
sha->update("some message");
auto message_hash = sha->final<MessageHash>();

@reneme
Chore: Buffered_Computation can deal with generic containers
d6f17f7 
This should be a transparent change for the most part but
enables modern support for all sorts of containers without
resorting to the C-style pointer APIs
reneme and others added 3 commits February 16, 2023 17:34
@reneme @lieser
concepts::resizeable_container requires T(size_t) c'tor
aa512d7 
Co-Authored-By: Philippe Lieser <philippe.lieser@rohde-schwarz.com>
@reneme @lieser
container StrongType offers StrongType(size_t) c'tor
7c4634a 
Co-Authored-By: Philippe Lieser <philippe.lieser@rohde-schwarz.com>
@reneme @lieser
Buffered_Computation with additional conveniece API
c4582aa 
Co-Authored-By: Philippe Lieser <philippe.lieser@rohde-schwarz.com>
@reneme
Copy link
Copy Markdown
Collaborator Author

reneme commented Feb 16, 2023

Sorry, I should have marked this as a draft while I was still working on it.

We added another overload for ::process<>() that allows custom output containers (same as ::final<>()). And also there's a test now, that makes sure all this convenience is actually working (aka compiling).

@reneme reneme requested a review from randombit February 16, 2023 16:42
@reneme
Copy link
Copy Markdown
Collaborator Author

reneme commented Feb 16, 2023

I deliberately left the virtual methods (below) untouched to get the low hanging fruits first.

virtual void add_data(const uint8_t input[], size_t length) = 0;
virtual void final_result(uint8_t out[]) = 0;

Though, both are good candidates for std::span<>. Particularly with sub-classes being forced to trust their base class(s) and write into an unbounded array. 😨

What's your take on this? Should we take this on?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 16, 2023

Codecov Report

Base: 88.08% // Head: 88.09% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (c4582aa) compared to base (e1b3226).
Patch coverage: 98.30% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3294      +/-   ##
==========================================
+ Coverage   88.08%   88.09%   +0.01%     
==========================================
  Files         610      611       +1     
  Lines       68481    68577      +96     
  Branches     6830     6832       +2     
==========================================
+ Hits        60322    60414      +92     
- Misses       5303     5310       +7     
+ Partials     2856     2853       -3
Impacted Files Coverage Δ
src/tests/test_bufcomp.cpp 98.30% <98.30%> (ø)
src/lib/pbkdf/argon2/argon2pwhash.cpp 69.69% <0.00%> (-9.10%) ⬇️
src/lib/utils/cpuid/cpuid_x86.cpp 54.05% <0.00%> (-0.62%) ⬇️
src/lib/math/numbertheory/numthry.cpp 83.81% <0.00%> (-0.58%) ⬇️
src/lib/tls/tls12/tls_record.cpp 91.80% <0.00%> (-0.41%) ⬇️
src/lib/block/aes/aes_ni/aes_ni.cpp 99.80% <0.00%> (-0.20%) ⬇️
src/lib/math/numbertheory/dsa_gen.cpp 89.28% <0.00%> (-0.19%) ⬇️
src/tests/test_hash.cpp 84.96% <0.00%> (-0.10%) ⬇️
src/tests/test_bigint.cpp 90.81% <0.00%> (-0.03%) ⬇️
src/lib/block/sm4/sm4.cpp 100.00% <0.00%> (ø)
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@randombit
Copy link
Copy Markdown
Owner

randombit commented Feb 16, 2023

What's your take on this? Should we take this on?

Sure, using span internally here would be a lot cleaner overall. But also a lot of work since you'll have to touch every hash/MAC. Those kind of changes tend to be a slog.

This was referenced Feb 17, 2023
Merged
Merged
Open
@reneme
Copy link
Copy Markdown
Collaborator Author

reneme commented Feb 24, 2023

@randombit I didn't merge this yet because I pushed some API changes after you approved the PR. Most notably: c4582aa

@reneme reneme mentioned this pull request Mar 16, 2023
Merged
randombit
randombit approved these changes Mar 16, 2023
View reviewed changes
Copy link
Copy Markdown
Owner

@randombit randombit left a comment

Choose a reason for hiding this comment

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

👍

@reneme reneme merged commit 8460ca2 into master Mar 17, 2023
@randombit randombit deleted the chore/generic_interface_bufcomp branch March 21, 2023 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@randombit randombit randombit approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@reneme @codecov-commenter @randombit