On 18 April 2016 at 12:43, Vinnie Falco wrote:

Greetings everyone, I'm new to the boost development mailing list.

I'm preparing a library for boost review, and it needs a SHA1 implementation. I have my own header only version I took from the net, based on https://github.com/vog/sha1.

I see that there's already a SHA1 implementation already in the boost, its part of UUID: https://github.com/boostorg/uuid/blob/develop/include/boost/uuid/sha1.hpp

Its bad form to duplicate code, but there are two problems with the UUID implementation: its in a detail namespace, and it seems to be completely unoptimized and without the unrolling present in the commonly used version originally written by Steve Reid (linked above).

What should I do? I'm guessing, some combination of the following:

1. Use UUID SHA1 as-is, even though its in a detail namespace.

2. Submit a pull request to UUID to make the SHA1 class a public interface. The problem here is the bar for public interfaces is quite high. I envision endless bike shedding (is "processs_bytes" the right name?)

3. Additionally submit a pull request to UUID to use the more optimized version of SHA1

4. Use my own version in my library submission, make it a detail class.

Thanks

I think you can't do 1) because detail is allowed to change (like you suggest in 3). Maybe it's a good idea to release you version using the std::hash (or boost:hash) interface, although a separte library of common predefined hash function would be a more natural place for it?

On 2016-04-18 13:43, Vinnie Falco wrote:

Greetings everyone, I'm new to the boost development mailing list.

I'm preparing a library for boost review, and it needs a SHA1 implementation. I have my own header only version I took from the net, based on https://github.com/vog/sha1.

I see that there's already a SHA1 implementation already in the boost, its part of UUID: https://github.com/boostorg/uuid/blob/develop/include/boost/uuid/sha1.hpp

Its bad form to duplicate code, but there are two problems with the UUID implementation: its in a detail namespace, and it seems to be completely unoptimized and without the unrolling present in the commonly used version originally written by Steve Reid (linked above).

What should I do? I'm guessing, some combination of the following:

1. Use UUID SHA1 as-is, even though its in a detail namespace.

2. Submit a pull request to UUID to make the SHA1 class a public interface. The problem here is the bar for public interfaces is quite high. I envision endless bike shedding (is "processs_bytes" the right name?)

3. Additionally submit a pull request to UUID to use the more optimized version of SHA1

4. Use my own version in my library submission, make it a detail class.

IMHO, SHA1 should not be a public part of Boost.UUID. I would rather have a separate library with cryprographic hash functions (and maybe not only them). Regarding your library, I would recommend using your own implementation in the detail namespace or decompose your submission into two libraries - one with cryptographic functions that are generally useful and the other one with the rest. The latter will probably be more difficult to bring into Boost because it will mean two libraries have to pass the review and I expect crypto library to have a very rigorous attention. You might also want to submit a PR to Boost.UUID to use an optimized version of SHA1. Either through your new library with crypto algorithms (after it's accepted) or internally (can be done at any time). That will be of no use for your submission but may be useful for Boost.UUID users. I would strongly recommend against using implementation details of other libraries.