On 6 May 2010 07:54, Stewart, Robert <Robert.Stewart@sig.com> wrote:
Links from one section to the next (and the reverse) would be helpful.
Added.
Not everyone considering your library knows about cryptographic hashes and the terminology relating to them. [...] Listing reasons for having created the library should be part of a Rationale section, or similar.
I've added an Introduction defining terms and suggesting usages, and moved the history to a Rationale.
In the bullet 2 table, the SHA-256 policy should be sha2<256>, right?
Yes.
Why include CubeHash when it is merely a candidate and you recommend not using it? Including some of the others is useful merely because there are existing demands for their use. Is the same true of CubeHash?
I knew I needed to include MD5 and the SHAs as they seem to be the most used. They are, however, constructed in the same way out of block cyphers with similar implementations, which is part of the reason NIST is currently holding its SHA-3 competition. I included CubeHash to ensure that different styles of hashes would still fit nicely in the concepts. (CubeHash is not based around a block cypher at all.) I'd like to add more second-round SHA-3 candidates as well (such as the sponge-construction-based Keccak and the tweakable-block-cypher-based Skein), but decided to get input on what I had before implementing more models.
To what bits does, "All the bits in the provided values are used," refer? Did you mean something like, "All bits in the input range values are used?"
Yes. I've changed it to use your phrasing.
Do you support Boost.Range/RangeEx?
Yes. The documentation now has a "single-pass range of readable iterators".
I don't understand to what the Portability Note applies. [...] The Alternative may make more sense when you clarify the "all bits" part I raised above.
I've rephrased both these sections; Let me know whether the changes are any better.
Is "HashPolicy" a good name? Is "Hash" not enough? That is, the Hash Concept? You don't have "MessageDigestPolicy" or "StreamHashPolicy," so why "HashPolicy?"
I put "Policy" in that name to emphasize that its usual usage would be as a policy template argument, rather than providing any functions. I've spelt that out explicitly now, and changed the concept name to HashAlgorithm. I'd prefer not to use just "Hash" since some people refer to digests as hashes, so I think being more explicit would be better.
[...] link to that concept section [...]
Added.
StreamHash Concept
Notice that I removed the "<ValueBits>" part. I don't think that should be part of the concept name.
The thing is that a StreamHash that looks at the bottom 4 bits of a value in update_one is semantically very different one one that looks at all 8 bits, so I considered that important enough to be part of the concept name. It also simplifies the description of HashAlgorithm, since it needs to say somehow that stream_hash<4>::type is different from stream_hash<8>::type.
Rather than spelling out the operations that must be supported, why not list the concepts that must be supported, when reasonable: DefaultConstructible, CopyConstructible, CopyAssignable?
Done.
What does, "Provided models are accessible through HashPolicy models," mean?
I've changed that part to read, "Each HashAlgorithm model provides access to all its associated StreamHash models; Those StreamHash models are generally not accessible in other ways." It's not essential though, and could be removed if it's more confusing than helpful. Thanks for the extensive feedback, ~ Scott McMurray