Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17251 )
Change subject: [encryption] Add encryption support to Env ...................................................................... Patch Set 9: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/17251/9/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: http://gerrit.cloudera.org:8080/#/c/17251/9/src/kudu/util/env-test.cc@1272 PS9, Line 1272: size_t nit: consider using constexpr size_t for this and 'size2' below http://gerrit.cloudera.org:8080/#/c/17251/9/src/kudu/util/env-test.cc@1340 PS9, Line 1340: string scratch2; nit: it seems this isn't needed? http://gerrit.cloudera.org:8080/#/c/17251/9/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: http://gerrit.cloudera.org:8080/#/c/17251/9/src/kudu/util/env_posix.cc@597 PS9, Line 597: RETURN_NOT_OK_EVAL(DoEncryptV(kDummyEncryptionKey, offset, data, encrypted_data), : delete[] encrypted_buf); nit: just curious why using RETURN_NOT_OK_EVAL() is preferred to wrapping encrypted_buf into unique_ptr and then calling release() on the wrapper right after the line with RETURN_NOT_OK(DoEncryptV(...)); ? -- To view, visit http://gerrit.cloudera.org:8080/17251 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id5a81b5736a0c1970818dbbc4a234480476b8d41 Gerrit-Change-Number: 17251 Gerrit-PatchSet: 9 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 10 Sep 2021 19:24:17 +0000 Gerrit-HasComments: Yes
