[PATCH] [Support] Add MemoryBuffer::getFileSlice()

kledzik at apple.com kledzik at apple.com
Tue Oct 7 15:47:09 PDT 2014



================
Comment at: include/llvm/Support/MemoryBuffer.h:131
@@ +130,3 @@
+  static ErrorOr<std::unique_ptr<MemoryBuffer>>
+  getFileSlice(const Twine &Filename, uint64_t Offset, uint64_t Length);
+
----------------
rafael wrote:
> s/Length/MapSize/
> 
> Pass MapSize before Offset.
> 
> Alternatively, update getOpenFileSlice to have this order/name. My only preference is that they match.
Fixed to match getOpenFileSlice

================
Comment at: lib/Support/MemoryBuffer.cpp:335
@@ -327,1 +334,3 @@
 
+  // If we don't know the file size, use fstat to find out.  fstat on an open
+  // file descriptor is cheaper than stat on a random path.
----------------
rafael wrote:
> Why was it necessary to move this?
> 
> Currently if the user passes the MapSize and we don't require a null terminator, we never call stat in here, since we don't need the file size.
> 
> I now notice that we call stat in mapped_file_region, which is a regression from when we transitioned to mapped_file_region :-(
> 
> In any case , if we can avoid an extra stat we should.
My thought was that since getFileSlice() provides a MapSize but not FileSize, we need to determine the file size.  

But it seems we don't actually need the FileSize when mapping.  The stat() looks like it is there to set the FileSize, but it is really there to get the MapSize.

http://reviews.llvm.org/D5423






More information about the llvm-commits mailing list