Adar Dembo has posted comments on this change. Change subject: WIP: KUDU-463. Add checksumming to cfile ......................................................................
Patch Set 13: (1 comment) http://gerrit.cloudera.org:8080/#/c/6630/13/src/kudu/cfile/cfile_reader.cc File src/kudu/cfile/cfile_reader.cc: PS13, Line 466: if (FLAGS_cfile_verify_checksums) { : uint32_t expected_checksum = DecodeFixed32(checksum.data()); : uint32_t checksum_value = crc::Crc32c(block.data(), block.size()); : if (PREDICT_FALSE(checksum_value != expected_checksum)) { : return Status::Corruption( : Substitute("Checksum does not match at offset $0 size $1: $2 vs $3", : ptr.offset(), block.size(), checksum_value, expected_checksum)); : } : } > I think I've seen this block of code three times; can you factor it out int Maybe it would even be possible to incorporate the ReadV() too? Basically a function that takes as input the data Slice and: 1. Creates a slice vector to pass to ReadV(). 2. Checks if checksums are to be verified. 3. If so, creates a checksum buffer+slice and modifies the slice vector. 4. Calls ReadV(). 5. If checksums are to be verified, performs the verification. -- To view, visit http://gerrit.cloudera.org:8080/6630 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6756834cd7f27af258797a3654a95244abeb0976 Gerrit-PatchSet: 13 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
