You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "jpeach (via GitHub)" <gi...@apache.org> on 2023/08/01 00:32:58 UTC
[GitHub] [trafficserver] jpeach opened a new pull request, #10121: Remove global symbols for zero hashes.
jpeach opened a new pull request, #10121:
URL: https://github.com/apache/trafficserver/pull/10121
Remove the two global symbols for zero-value crypto hashes, and replace them with inline member functions. This is 2 less global symbols, and improves code readability (though marginally).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] masaori335 commented on a diff in pull request #10121: Remove global symbols for zero hashes.
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #10121:
URL: https://github.com/apache/trafficserver/pull/10121#discussion_r1282589167
##########
include/tscore/CryptoHash.h:
##########
@@ -107,9 +109,20 @@ union CryptoHash {
/// Fast conversion to hex in fixed sized string.
char *toHexStr(char buffer[(CRYPTO_HASH_SIZE * 2) + 1]) const;
-};
-extern CryptoHash const CRYPTO_HASH_ZERO;
+ bool
+ is_zero() const
+ {
+ const uint8_t z64[sizeof(u64)] = {0};
Review Comment:
nitpicking: would you make this `constexpr`? because `sizeof(u64)` is defined in the compile time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] jpeach commented on pull request #10121: Remove global symbols for zero hashes.
Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on PR #10121:
URL: https://github.com/apache/trafficserver/pull/10121#issuecomment-1666355986
@masaori335 Can you pls approve this again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] jpeach merged pull request #10121: Remove global symbols for zero hashes.
Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach merged PR #10121:
URL: https://github.com/apache/trafficserver/pull/10121
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] jpeach commented on a diff in pull request #10121: Remove global symbols for zero hashes.
Posted by "jpeach (via GitHub)" <gi...@apache.org>.
jpeach commented on code in PR #10121:
URL: https://github.com/apache/trafficserver/pull/10121#discussion_r1283943319
##########
include/tscore/CryptoHash.h:
##########
@@ -107,9 +109,20 @@ union CryptoHash {
/// Fast conversion to hex in fixed sized string.
char *toHexStr(char buffer[(CRYPTO_HASH_SIZE * 2) + 1]) const;
-};
-extern CryptoHash const CRYPTO_HASH_ZERO;
+ bool
+ is_zero() const
+ {
+ const uint8_t z64[sizeof(u64)] = {0};
Review Comment:
Done!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org