Skip to content

Commit

Permalink
Fix non-exported memfiles.setFileSize to be able to shrink files on…
Browse files Browse the repository at this point in the history
… posix via `memfiles.resize` (#23717)

Fix non-exported `setFileSize` to take optional `oldSize` to (on posix)
shrink differently than it grows (`ftruncate` not `posix_fallocate`)
since it makes sense to assume the higher address space has already been
allocated there and include the old file size in the `proc resize` call.
Also, do not even try `setFileSize` in the first place unless the `open`
itself works by moving the call into the `if newFileSize != -1` branch.

Just cosmetics, also improve some old 2011 comments, note a logic diff
for callers using both `mappedSize` & `newFileSize` from windows branch
in case someone wants to fix that & simplify code formatting a little.
  • Loading branch information
c-blake authored Jun 14, 2024
1 parent 0b5a938 commit 8037bbe
Showing 1 changed file with 30 additions and 36 deletions.
66 changes: 30 additions & 36 deletions lib/pure/memfiles.nim
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ proc newEIO(msg: string): ref IOError =
new(result)
result.msg = msg

proc setFileSize(fh: FileHandle, newFileSize = -1): OSErrorCode =
## Set the size of open file pointed to by `fh` to `newFileSize` if != -1.
## Space is only allocated if that is cheaper than writing to the file. This
## routine returns the last OSErrorCode found rather than raising to support
## old rollback/clean-up code style. [ Should maybe move to std/osfiles. ]
if newFileSize == -1:
proc setFileSize(fh: FileHandle, newFileSize = -1, oldSize = -1): OSErrorCode =
## Set the size of open file pointed to by `fh` to `newFileSize` if != -1,
## allocating | freeing space from the file system. This routine returns the
## last OSErrorCode found rather than raising to support old rollback/clean-up
## code style. [ Should maybe move to std/osfiles. ]
if newFileSize < 0 or newFileSize == oldSize:
return
when defined(windows):
var sizeHigh = int32(newFileSize shr 32)
Expand All @@ -51,14 +51,18 @@ proc setFileSize(fh: FileHandle, newFileSize = -1): OSErrorCode =
setEndOfFile(fh) == 0:
result = lastErr
else:
var e: cint # posix_fallocate truncates up when needed.
when declared(posix_fallocate):
while (e = posix_fallocate(fh, 0, newFileSize); e == EINTR):
discard
if e in [EINVAL, EOPNOTSUPP] and ftruncate(fh, newFileSize) == -1:
result = osLastError() # fallback arguable; Most portable, but allows SEGV
elif e != 0:
result = osLastError()
if newFileSize > oldSize: # grow the file
var e: cint # posix_fallocate truncates up when needed.
when declared(posix_fallocate):
while (e = posix_fallocate(fh, 0, newFileSize); e == EINTR):
discard
if e in [EINVAL, EOPNOTSUPP] and ftruncate(fh, newFileSize) == -1:
result = osLastError() # fallback arguable; Most portable BUT allows SEGV
elif e != 0:
result = osLastError()
else: # shrink the file
if ftruncate(fh.cint, newFileSize) == -1:
result = osLastError()

type
MemFile* = object ## represents a memory mapped file
Expand Down Expand Up @@ -255,41 +259,31 @@ proc open*(filename: string, mode: FileMode = fmRead,
flags = flags or O_CREAT or O_TRUNC
var permissionsMode = S_IRUSR or S_IWUSR
result.handle = open(filename, flags, permissionsMode)
if result.handle != -1:
if (let e = setFileSize(result.handle.FileHandle, newFileSize);
e != 0.OSErrorCode): fail(e, "error setting file size")
else:
result.handle = open(filename, flags)

if result.handle == -1:
# XXX: errno is supposed to be set here
# Is there an exception that wraps it?
fail(osLastError(), "error opening file")

if (let e = setFileSize(result.handle.FileHandle, newFileSize);
e != 0.OSErrorCode): fail(e, "error setting file size")

if mappedSize != -1:
result.size = mappedSize
else:
var stat: Stat
if mappedSize != -1: #XXX Logic here differs from `when windows` branch ..
result.size = mappedSize #.. which always fstats&Uses min(mappedSize, st).
else: # if newFileSize!=-1: result.size=newFileSize # if trust setFileSize
var stat: Stat #^^.. BUT some FSes (eg. Linux HugeTLBfs) round to 2MiB.
if fstat(result.handle, stat) != -1:
# XXX: Hmm, this could be unsafe
# Why is mmap taking int anyway?
result.size = int(stat.st_size)
result.size = stat.st_size.int # int may be 32-bit-unsafe for 2..<4 GiB
else:
fail(osLastError(), "error getting file size")

result.flags = if mapFlags == cint(-1): MAP_SHARED else: mapFlags
#Ensure exactly one of MAP_PRIVATE cr MAP_SHARED is set
# Ensure exactly one of MAP_PRIVATE cr MAP_SHARED is set
if int(result.flags and MAP_PRIVATE) == 0:
result.flags = result.flags or MAP_SHARED

result.mem = mmap(
nil,
result.size,
if readonly: PROT_READ else: PROT_READ or PROT_WRITE,
result.flags,
result.handle,
offset)

let pr = if readonly: PROT_READ else: PROT_READ or PROT_WRITE
result.mem = mmap(nil, result.size, pr, result.flags, result.handle, offset)
if result.mem == cast[pointer](MAP_FAILED):
fail(osLastError(), "file mapping failed")

Expand Down Expand Up @@ -353,7 +347,7 @@ proc resize*(f: var MemFile, newFileSize: int) {.raises: [IOError, OSError].} =
raise newException(IOError,
"Cannot resize MemFile opened with allowRemap=false")
if newFileSize != f.size:
if (let e = setFileSize(f.handle.FileHandle, newFileSize);
if (let e = setFileSize(f.handle.FileHandle, newFileSize, f.size);
e != 0.OSErrorCode): raiseOSError(e)
when defined(linux): #Maybe NetBSD, too?
# On Linux this can be over 100 times faster than a munmap,mmap cycle.
Expand Down

0 comments on commit 8037bbe

Please sign in to comment.