[PATCH] D66471: [Support] Improve readNativeFile(Slice) interface

[Pavel Labath via Phabricator via llvm-commits]

labath marked an inline comment as done.
labath added inline comments.


================
Comment at: lib/Support/MemoryBuffer.cpp:467
+    Expected<size_t> ReadBytes =
+        sys::fs::readNativeFileSlice(FD, Buf->getBuffer(), Offset);
+    if (!ReadBytes)
----------------
aganea wrote:
> labath wrote:
> > aganea wrote:
> > > Shouldn't we use `ToRead` here instead of `Buf->getBuffer()`? Could you please test two sequential reads in the unittests?
> > Woops, indeed it should. Thanks for catching that!
> > 
> > As for the test, I am not sure how I could force the read syscall to return partial data here. For the non-slice version, I suppose I could make this read from a pipe, and then make sure I write to the pipe very slowly. However, the slice version requires a seekable fd, and I don't know what to do there... Do you have any suggestion?
> You can probably call `MemoryBuffer::getOpenFile` with a smaller `FileSize` & `MapSize` and `IsVolatile = true`? And then a subsequent call by setting `Offset` > 0? Would that work?
I don't fully understand what you mean, but I don't see how could that work. Are you expecting that the second time around the OS will return the partial read first, because that data will already be cached and ready? If so, then I would be very surprised if that works, particularly for a file that has just been created because it will likely be present in the disk cache in its entirety.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66471/new/

https://reviews.llvm.org/D66471