> On Oct. 13, 2015, 12:54 a.m., Jiang Yan Xu wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, lines 85-120
> > <https://reviews.apache.org/r/38747/diff/14/?file=1092722#file1092722line85>
> >
> > These templates (here and below) have different levels of
> > specializations, traits, typedefs in various places, which is hard to
> > understand.
> >
> > Do you think the following is simpler?
> >
> > For each digest type, there is one **low-level** implementation, i.e.,
> > Init, Update and Final are called separatedly but they expose one common
> > interface (without digest-type specific) arguments.
> >
> > ```
> > class DigestImpl
> > {
> > public:
> > void init() == 0;
> > int update(const void*, size_t) == 0;
> > int final(uint8_t*) == 0;
> > static Try<DigestImpl*> create(...);
> > };
> > ```
> >
> > You have a low implementation for each type that inherits this
> > interface and encapsulates its specific context variables in its member
> > variables. The low-level implementation should be simply calling openssl
> > APIs so it should be short and has no more redundant code among different
> > implementations than specializations in different places and this
> > consolidates handling of specific digest-types in one single place.
> >
> > The high-level DigestUtil implementation or simply, the static generic
> > method implementation can just create a low-level impl class and you can
> > put common logic there.
> >
> > What do you think?
I should probably try to reason about the choice of templates (as opposed to
runtime polymorphism)
SSL digest API can be seen as "logic" and "structure". The logic is the same
for all SHAs. Only structure is different. The difference in structure can be
represented as template traits. This allows the difference between SHAs to be
viewed as *declarative* (as opposed to *procedural*).
This has following advantages:
1. Any new SHA digest can be added by just adding the type traits
*declaratively*. For example, to add SHA224, we just need to add the following
lines:
```
template <>
struct DigestTypeTraits<Digest::SHA224>
{
typedef SHA224_CTX ctx_type;
static const size_t digest_length = SSHA224_DIGEST_LENGTH;
};
template <>
DigestFunctionTraits<Digest::SHA224>::Init_fn
DigestFunctionTraits<Digest::SHA256>::Init = SHA224_Init;
template <>
DigestFunctionTraits<Digest::SHA224>::Update_fn
DigestFunctionTraits<Digest::SHA256>::Update = SHA224_Update;
template <>
DigestFunctionTraits<Digest::SHA224>::Final_fn
DigestFunctionTraits<Digest::SHA224>::Final = SHA224_Final;
```
If you notice all of the above new code is declarative and does not add any new
logic. This is possible because we have abstracted out the difference between
SHA implementations into templates.
2. At the call site, clients can call digests simply by:
```
DigestUtil<Digest::SHA256>::digest(string)
```
This avoids runtime polymorphism.
> On Oct. 13, 2015, 12:54 a.m., Jiang Yan Xu wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, lines 159-160
> > <https://reviews.apache.org/r/38747/diff/14/?file=1092722#file1092722line159>
> >
> > About templatization. I previously commented that the implementation
> > should be put in the CPP file.
> >
> > You use templates but your interafce is not templatized and used only
> > by this single component. Can't all the templates just be in the same
> > source file as the caller so that the header only has the API declarations?
> >
> > This of course becomes a moot point is we don't use template as I was
> > suggesting above.
One of the reasons the templates are in header file is to allow client code
like:
```
DigestUtil<Digest::SHA256>::digest(string)
```
- Jojy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38747/#review102302
-----------------------------------------------------------
On Oct. 12, 2015, 9:14 p.m., Jojy Varghese wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38747/
> -----------------------------------------------------------
>
> (Updated Oct. 12, 2015, 9:14 p.m.)
>
>
> Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added SHA256, SHA512 implementation.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am eb7393904988afc0bc5e9f7dadd545e0d6c04e5f
> 3rdparty/libprocess/include/Makefile.am
> fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e
> 3rdparty/libprocess/include/process/digest.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927
> 3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/38747/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jojy Varghese
>
>
