Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23429 )

Change subject: OpenSSL 3.x compatibility adaptation
......................................................................


Patch Set 12:

(2 comments)

Thank you for the update!  Now this can be successfully built on my ubuntu 
24.04 LTS build machine.

This patch now looks good to me, there is just one question and a request to 
move in-line documentation, if the change in SCOPED_OPENSSL_NO_PENDING_ERRORS 
location is indeed still necessary.

http://gerrit.cloudera.org:8080/#/c/23429/12/src/kudu/util/openssl_util.h
File src/kudu/util/openssl_util.h:

http://gerrit.cloudera.org:8080/#/c/23429/12/src/kudu/util/openssl_util.h@68
PS12, Line 68: // Scoped helper which DCHECKs that on both scope entry and 
exit, there are no
             : // pending OpenSSL errors for the current thread.
             : //
             : // This allows us to avoid calling ERR_clear_error() defensively 
before every
             : // OpenSSL call, but rather call it only when we get an error 
code indicating
             : // there may be some pending error.
             : //
             : // Example usage:
             : //
             : //    void MyFunc() {
             : //      SCOPED_OPENSSL_NO_PENDING_ERRORS;
             : //      ... use OpenSSL APIs ...
             : //    }
If it's indeed necessary to move SCOPED_OPENSSL_NO_PENDING_ERRORS, then please 
move this comment to be next to the new location of the 
SCOPED_OPENSSL_NO_PENDING_ERRORS.  Otherwise, it's not easy to comprehend what 
exact 'scoped helper' is described in this comment, and the macro is now left 
without proper documentation.


http://gerrit.cloudera.org:8080/#/c/23429/12/src/kudu/util/openssl_util.h@283
PS12, Line 283: avoiding the need for a forward declaration
I'm curious: what was the problem with the original location of this macro?  
AFAIK, macros usually don't need any forward declarations since they only 
evaluated after being substituted, but I might be missing something.

Are you sure it's still necessary to move this macro from its original location 
after adding appropriate IWYU pragmas in PS12?



--
To view, visit http://gerrit.cloudera.org:8080/23429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic587a85e6b9088ffd353f9119b75431f1ec60b5c
Gerrit-Change-Number: 23429
Gerrit-PatchSet: 12
Gerrit-Owner: Yan-Daojiang <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yan-Daojiang <[email protected]>
Gerrit-Comment-Date: Wed, 15 Oct 2025 06:24:18 +0000
Gerrit-HasComments: Yes

Reply via email to