Grant Henke has posted comments on this change. Change subject: env: Always read fully when reading files ......................................................................
Patch Set 2: > In many places, env_util.h was only included for ReadFully(). Wherever you're > replacing ReadFully() with Read(), could you check if that was the last thing > that requires env_util.h, and if so, remove it? I did this, but I will go back through and double check again. > With short reads out of the picture, I wonder if we can simplify the Read() > API a bit. As per the comment I left in your checksumming patch, having both > the scratch/len and the slice seems redundant; one or the other should > suffice to fully describe the read input and output. I think the scratch and length are decoupled so that you can use a scratch that is larger than the request length you plan to read. This allows you to allocate a scratch and fill it with multiple reads. -- 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: 2 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-HasComments: No
