Adar Dembo has posted comments on this change. Change subject: env: add ReadV() API ......................................................................
Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/6779/6/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: PS6, Line 475: WriteTestFile(env, kTestPath, data_size); : ASSERT_NO_FATAL_FAILURE(); You can combine this: NO_FATALS(WriteTestFile(...)); Line 487: results.push_back(Slice(scratch + shift, slice_size)); You can do: results.emplace_back({scratch + shift, slice_size}); Or maybe it's without the braces; I can't remember. PS6, Line 496: // Turn short reads off again : FLAGS_env_inject_short_read_bytes = 0; If the goal here is to reset the gflag for the next test, you don't need to worry about it. Every KuduTest defines a google::FlagSaver which restores all gflags to their default values when the test ends. http://gerrit.cloudera.org:8080/#/c/6779/2/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 330: uint64_t cur_offset = offset; > That doesn't seam to work. I can do the following though: Right, I forgot that results is a pointer to a vector. Would this work? Slice& result = (*results)[i]; This way you don't have to make a copy of the slice. http://gerrit.cloudera.org:8080/#/c/6779/6/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 371: bytes_rem -= bytes_rem; Once we hit this case, shouldn't we break out of the loop? If bytes_rem < iov[i].iov_len, that means bytes_rem will be 0 for every remaining iteration of the loop, and L369-371 will be no-ops. -- To view, visit http://gerrit.cloudera.org:8080/6779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib8f7a62c8363b40baa064d9e63be1ece506f1e48 Gerrit-PatchSet: 6 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-HasComments: Yes
