Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17251 )
Change subject: [encryption] Add encryption support to Env ...................................................................... Patch Set 10: Code-Review+2 (1 comment) 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); > iov is used below in pwritev and it points to encrypted_buf's memory area, That's right, but from what I can see these two snippets are equivalent in addressing that, no? if (encrypt) { ... unsigned char* encrypted_buf = new unsigned char[bytes_req]; ... RETURN_NOT_OK_EVAL(DoEncryptV(...), delete[] encrypted_buf); } else { ... } and if (encrypt) { ... unique_ptr<unsigned char[]> encrypted_buf(new unsigned char[bytes_req]); ... RETURN_NOT_OK(DoEncryptV(...)); encrypted_buf.release(); } else { ... } Anyways, the current version looks good to me as well, so feel free to leave it as is. -- 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: 10 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: Sat, 11 Sep 2021 00:58:37 +0000 Gerrit-HasComments: Yes
