Adar Dembo has posted comments on this change.

Change subject: KUDU-1377 (part 2): Add version 2 of protobuf container file
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/2815/2/src/kudu/util/pb_util.cc
File src/kudu/util/pb_util.cc:

Line 276:   Slice length_checksum(len_and_cksum_slice.data() + 
sizeof(uint32_t), kPBContainerChecksumLen);
Move this to within the version >= 2 check, since it doesn't make sense 
otherwise.


Line 277:   uint32_t data_length = DecodeFixed32(length.data());
Maybe defer this until the checksum is checked? Nothing bad will happen if the 
length was corrupt and we decode it, but deferring it will adhere to the 
overall order of operations better.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I239c7b99a55a74a6a658ff418830c1f9be13eaa6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to