Copilot commented on code in PR #49780:
URL: https://github.com/apache/arrow/pull/49780#discussion_r3175343792


##########
cpp/src/gandiva/hash_utils.h:
##########
@@ -37,11 +37,6 @@ GANDIVA_EXPORT
 const char* gdv_sha1_hash(int64_t context, const void* message, size_t 
message_length,
                           int32_t* out_length);
 
-GANDIVA_EXPORT
-const char* gdv_hash_using_openssl(int64_t context, const void* message,
-                                   size_t message_length, const EVP_MD* 
hash_type,
-                                   uint32_t result_buf_size, int32_t* 
out_length);
-
 GANDIVA_EXPORT
 const char* gdv_md5_hash(int64_t context, const void* message, size_t 
message_length,
                          int32_t* out_length);

Review Comment:
   Now that `gdv_hash_using_openssl` is no longer declared in this header, 
`hash_utils.h` no longer appears to require any OpenSSL types. Consider 
dropping the `openssl/evp.h` include from this header and keeping OpenSSL 
includes in the .cc to avoid imposing an unnecessary public header dependency 
on OpenSSL.



##########
cpp/src/gandiva/hash_utils.cc:
##########
@@ -21,49 +21,12 @@
 #include "gandiva/gdv_function_stubs.h"
 #include "openssl/evp.h"
 
-namespace gandiva {
-
-/// Hashes a generic message using the SHA512 algorithm
-GANDIVA_EXPORT
-const char* gdv_sha512_hash(int64_t context, const void* message, size_t 
message_length,
-                            int32_t* out_length) {
-  constexpr int sha512_result_length = 128;
-  return gdv_hash_using_openssl(context, message, message_length, EVP_sha512(),
-                                sha512_result_length, out_length);
-}
-
-/// Hashes a generic message using the SHA256 algorithm
-GANDIVA_EXPORT
-const char* gdv_sha256_hash(int64_t context, const void* message, size_t 
message_length,
-                            int32_t* out_length) {
-  constexpr int sha256_result_length = 64;
-  return gdv_hash_using_openssl(context, message, message_length, EVP_sha256(),
-                                sha256_result_length, out_length);
-}
-
-/// Hashes a generic message using the SHA1 algorithm
-GANDIVA_EXPORT
-const char* gdv_sha1_hash(int64_t context, const void* message, size_t 
message_length,
-                          int32_t* out_length) {
-  constexpr int sha1_result_length = 40;
-  return gdv_hash_using_openssl(context, message, message_length, EVP_sha1(),
-                                sha1_result_length, out_length);
-}
-
-GANDIVA_EXPORT
-const char* gdv_md5_hash(int64_t context, const void* message, size_t 
message_length,
-                         int32_t* out_length) {
-  constexpr int md5_result_length = 32;
-  return gdv_hash_using_openssl(context, message, message_length, EVP_md5(),
-                                md5_result_length, out_length);
-}
-
+namespace {
 /// \brief Hashes a generic message using SHA algorithm.
 ///
 /// It uses the EVP API in the OpenSSL library to generate
 /// the hash. The type of the hash is defined by the
 /// \b hash_type \b parameter.

Review Comment:
   The helper comment says it hashes using a "SHA algorithm", but the same 
helper is also used for MD5 (and potentially other digests). Updating the 
wording to something like "hash/digest algorithm" would keep the documentation 
accurate.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to