https://bitbucket.org/guenther/openclonk/changeset/52fddb70be7f
https://bitbucket.org/guenther/openclonk/changeset/6a506b62472e
Is anybody bothered enough by the OpenSSL dependency to check whether exchanging game data during the network lobby still works with these two patches? I was just bothered enough to write them, but not enough to test them.
https://bitbucket.org/guenther/openclonk/changeset/6a506b62472e
Is anybody bothered enough by the OpenSSL dependency to check whether exchanging game data during the network lobby still works with these two patches? I was just bothered enough to write them, but not enough to test them.
On which OS should it be tested? Only downloading game data from the host shall be tested?
Should be platform-independent. Network "resources" and league records are the only users of the sha1 checksum feature. Checking whether the removal of C4ConfigShareware broke anything would be good as well, but that's so broad a question that it's probably not worth testing separately.
Are we sure we want to remove it? I think we might still have some use for it in future... say associating each key with a private key so they can be used for signing groups or secure authentication in network or league games. If (!) we ever manage to get league play to work with OC, it will probably involve more crypto, not less.
I don't agree with your rationale to copy&paste a header just to remove the namespace. A typedef would have sufficed.
<ck> you copy the code just because it is inside a namespace? Come on o_O
<Guenther> well, the real reason is that boost/uuid/details seems to want to tell me not to use it
<Guenther> also, it's not documented whereas the other contents of uuid are
<ck> then maybe you should state that reason in the commit message
<Guenther> perhaps. but typing sha1 instead of boost::uuid::details::sha1 is enough reason on its own, and only people who love C++ namespaces would disagree ;-)
<Isilkor> you do know that using declarations exist
<ck> no, it's not enough reason for not using the system boost implementation
<ck> you can write a wrapper if you don't like the namespace
<Guenther> oh, and I needed to add a #define SHA_DIGEST_LENGTH 20 to avoid changing the Clonk code too much
<Guenther> I briefly considered also changing the SHA1 implementation to avoid that cast in c4group
<Guenther> Hm, though I now wonder whether the cast works
So you need to do the necessary research to find out whether boost::uuid::detail is stable enough for our purposes if you want to avoid the code duplication. Given that it's not necessary for the documented parts of the uuid API, the code is a pure C++ implementation of a function that will never change, the apparent risk of us missing a vital update is low compared to the risk of boost changing the uuid details. Also, it's a header-only implementation, so most of the usual arguments against code duplication do not apply.
<Guenther> well, the real reason is that boost/uuid/details seems to want to tell me not to use it
<Guenther> also, it's not documented whereas the other contents of uuid are
<ck> then maybe you should state that reason in the commit message
<Guenther> perhaps. but typing sha1 instead of boost::uuid::details::sha1 is enough reason on its own, and only people who love C++ namespaces would disagree ;-)
<Isilkor> you do know that using declarations exist
<ck> no, it's not enough reason for not using the system boost implementation
<ck> you can write a wrapper if you don't like the namespace
<Guenther> oh, and I needed to add a #define SHA_DIGEST_LENGTH 20 to avoid changing the Clonk code too much
<Guenther> I briefly considered also changing the SHA1 implementation to avoid that cast in c4group
<Guenther> Hm, though I now wonder whether the cast works
So you need to do the necessary research to find out whether boost::uuid::detail is stable enough for our purposes if you want to avoid the code duplication. Given that it's not necessary for the documented parts of the uuid API, the code is a pure C++ implementation of a function that will never change, the apparent risk of us missing a vital update is low compared to the risk of boost changing the uuid details. Also, it's a header-only implementation, so most of the usual arguments against code duplication do not apply.
Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill