Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18025 )
Change subject: [security] KUDU-3316 Add encrypted file keys ...................................................................... Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/18025/9/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: http://gerrit.cloudera.org:8080/#/c/18025/9/src/kudu/util/env_posix.cc@1196 PS9, Line 1196: retur style nit here and elsewhere with overridden virtual methods: drop 'virtual' since 'override' is already enough to express that the method is virtual. Not sure whether it's better to do so in this or in a separate changelist since there are other methods in those interfaces which have extra 'virtual' fluff along with the 'override' attribute' http://gerrit.cloudera.org:8080/#/c/18025/8/src/kudu/util/file_cache-test.cc File src/kudu/util/file_cache-test.cc: http://gerrit.cloudera.org:8080/#/c/18025/8/src/kudu/util/file_cache-test.cc@103 PS8, Line 103: unique_ptr<RWFile> f; : RWFileOptions opts; : opts.is_sensitive = true; : RETURN_NOT_OK(env_->NewRWFile(opts, name, &f)); : RETURN_NOT_OK(f->Write(f->GetEncryptionHeaderSize(), data)); > PS3 was like this, in that version I add the header size to the offset on r I don't see that PS3 implements the proposed approach -- the decision whether to add or not the offset is always sort of manual. But frankly, I didn't try implementing the proposed approach myself, so most likely I'm missing a few important points there (maybe that's about the difference between logical/on-disk/etc sizes, etc.). OK, if you are sure those issues cannot be handled properly by the approach suggested, I'm fine with the alternative once you are sure the code is more maintainable this way. -- To view, visit http://gerrit.cloudera.org:8080/18025 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idb1282c117271fda63a8cc54c00add7cc96dcffd Gerrit-Change-Number: 18025 Gerrit-PatchSet: 8 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: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Fri, 14 Jan 2022 06:20:27 +0000 Gerrit-HasComments: Yes
