[PATCH] D63453: [Support] Move llvm::MemoryBuffer to sys::fs::file_t
Pavel Labath via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 16 06:19:11 PDT 2019
labath added inline comments.
================
Comment at: llvm/trunk/lib/Support/Windows/Path.inc:1258-1262
+ // Once we reach EOF, zero the remaining bytes in the buffer.
+ if (BytesRead == 0) {
+ memset(BufPtr, 0, BytesLeft);
+ break;
+ }
----------------
BTW, I've was looking at this code in context of D66344, and I have found this quirk of this API to be very surprising and unexpected. Zeroing the remainder of the buffer may be the right behavior for `MemoryBuffer::getFileSlice` (though I'm not even convinced of that), but I definitely wouldn't expect that to happen in `readNativeFileSlice`. I think that an additional `size_t *BytesRead` read argument would be more natural here (and consistent with `readNativeFile`).
What would you say to a patch which adds a BytesRead argument to this function and moves the zeroing code to the MemoryBuffer class (currently the only user of this function)?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63453/new/
https://reviews.llvm.org/D63453
More information about the llvm-commits
mailing list