Conversation
| template<typename DerivedT, mdx_hash_implementation HashImplT> | ||
| class MD_Hash_Adapter : public HashFunction | ||
| { | ||
| public: | ||
| std::string name() const override { return HashImplT::NAME; } | ||
| size_t output_length() const override { return HashImplT::FINAL_DIGEST_BYTES; } | ||
| size_t hash_block_size() const override { return HashImplT::BLOCK_BYTES; } | ||
|
|
||
| std::unique_ptr<HashFunction> new_object() const override { return std::make_unique<DerivedT>(); } | ||
| std::unique_ptr<HashFunction> copy_state() const override { return std::make_unique<DerivedT>(*dynamic_cast<const DerivedT*>(this)); } | ||
|
|
||
| void clear() override { m_md.clear(); } | ||
|
|
||
| private: | ||
| void add_data(const uint8_t input[], size_t length) override { m_md.add_data(input, length); } | ||
| void final_result(uint8_t output[]) override { m_md.final_result(output); } | ||
|
|
||
| private: | ||
| MD_Hash<HashImplT> m_md; | ||
| }; |
There was a problem hiding this comment.
Obviously, this is a departure from the idea that inheritance shouldn't be used for implementation but merely to communicate interface relationship.
However, I'd like to argue that this nicely separates the API-concerns of HashFunction from the implementation of MDx-hashes (::init(), compress_n() and their specific parameters). The implementations don't need to worry about the top-level interface at all (except for the provider() method when ISA-specific implementations are available).
Separating MD_Hash and MD_Hash_Adapter could even be reverted (as the latter is really just a wrapper, IMO). Though, also thinking about Keccak[c]: a similar separation would likely allow further code reuse via additional adapters such as KMAC or XOFs.
@randombit @falko-strenzke Is that something we could work with?
There was a problem hiding this comment.
I fear I don't capture the whole design idea for MD_Hash from this sketch. In any case, I don't agree with the principle that inheritance should only be used for interfaces but not for implementation. If the latter doesn't bring the risk of an increased complexity and difficult maintenance but allows to reduce code duplication, it is fine in my view.
I tried capturing the design idea as a class diagram (click to enlarge).
|
|
To me this design makes sense and would probably be transferable to the Keccak-derived hash- and XOF functions. |
|
Now that we are going with the pure membership approach over an adapter base class, I'll close this. Let's try to move forward with the MDx and Keccak refactorings. :) |
That's still somewhat sketchy, but I think it makes my point clear about having a CRTP-Adapter to move shared code (i.e.
HashFunctioninterface implementation) into a central location. As I said: That's not a must at all. Albeit, I do believe it cleans things up quite nicely.