[PATCH] D63453: [Support] Move llvm::MemoryBuffer to sys::fs::file_t

[Pavel Labath via Phabricator via llvm-commits]

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