Adar Dembo has posted comments on this change. Change subject: env: Always read fully when reading files ......................................................................
Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/6758/3//COMMIT_MSG Commit Message: PS3, Line 9: KUDU-9(see ) Missing a link here? To be honest it's not necessary; KUDU-9 is descriptive enough. PS3, Line 13: implimented implemented Line 14: (see a15c795360e32885c00442efacd2a345f993f425) This is the same commit hash as above; one of them is probably wrong. PS3, Line 17: regardles Nit: regardless http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: Line 376: class ShortReadRandomAccessFile : public RandomAccessFile { Not using this anymore, can be removed? http://gerrit.cloudera.org:8080/#/c/6758/3/src/kudu/util/env_posix.cc File src/kudu/util/env_posix.cc: Line 255: static Status DoRead(int fd, const string& filename, uint64_t offset, size_t length, > warning: 'DoRead' is a static definition in anonymous namespace; static is Mind fixing this? All these static functions are already in an anonymous namespace, so they needn't be declared static. Line 273: Slice this_result(dst, r); Not really sure what 'this_result' adds; you could just as easily s/this_result.size()/r/ in the body of the loop. -- To view, visit http://gerrit.cloudera.org:8080/6758 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I735cd4cfca3c355226266ef4f0fdb57bf59dfe69 Gerrit-PatchSet: 3 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: Tidy Bot Gerrit-HasComments: Yes
