fix #17888: remove undefined behavior for posix.open; fix tempfiles.createTempFile (#17889)

* fix #17888: remove undefined behavior for posix.open; fix tempfiles.createTempFile

* fix for tests/async/tasyncfile.nim

* hide mode for now

* add notice regarding stability
This commit is contained in:
Timothee Cour 2021-04-29 04:42:56 -07:00 • committed by GitHub
commit e4db733d80
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 16 additions and 4 deletions

View file

@ -183,7 +183,11 @@ proc dlsym*(a1: pointer, a2: cstring): pointer {.importc, header: "<dlfcn.h>", s
proc creat*(a1: cstring, a2: Mode): cint {.importc, header: "<fcntl.h>", sideEffect.} proc creat*(a1: cstring, a2: Mode): cint {.importc, header: "<fcntl.h>", sideEffect.}
proc fcntl*(a1: cint | SocketHandle, a2: cint): cint {.varargs, importc, header: "<fcntl.h>", sideEffect.} proc fcntl*(a1: cint | SocketHandle, a2: cint): cint {.varargs, importc, header: "<fcntl.h>", sideEffect.}
proc open*(a1: cstring, a2: cint): cint {.varargs, importc, header: "<fcntl.h>", sideEffect.} proc openImpl(a1: cstring, a2: cint): cint {.varargs, importc: "open", header: "<fcntl.h>", sideEffect.}
proc open*(a1: cstring, a2: cint, mode: Mode | cint = 0.Mode): cint {.inline.} =
# prevents bug #17888
openImpl(a1, a2, mode)
proc posix_fadvise*(a1: cint, a2, a3: Off, a4: cint): cint {. proc posix_fadvise*(a1: cint, a2, a3: Off, a4: cint): cint {.
importc, header: "<fcntl.h>".} importc, header: "<fcntl.h>".}
proc posix_fallocate*(a1: cint, a2, a3: Off): cint {. proc posix_fallocate*(a1: cint, a2, a3: Off): cint {.

View file

@ -1503,6 +1503,7 @@ when false:
of fmReadWrite: flags = O_RDWR or int(O_CREAT) of fmReadWrite: flags = O_RDWR or int(O_CREAT)
of fmReadWriteExisting: flags = O_RDWR of fmReadWriteExisting: flags = O_RDWR
of fmAppend: flags = O_WRONLY or int(O_CREAT) or O_APPEND of fmAppend: flags = O_WRONLY or int(O_CREAT) or O_APPEND
static: doAssert false # handle bug #17888
var handle = open(filename, flags) var handle = open(filename, flags)
if handle < 0: raise newEOS("posix.open() call failed") if handle < 0: raise newEOS("posix.open() call failed")
result = newFileHandleStream(handle) result = newFileHandleStream(handle)

View file

@ -8,6 +8,8 @@
# #
## This module creates temporary files and directories. ## This module creates temporary files and directories.
##
## Experimental API, subject to change.
import os, random import os, random
@ -44,6 +46,8 @@ else:
proc safeOpen(filename: string): File = proc safeOpen(filename: string): File =
## Open files exclusively. ## Open files exclusively.
# xxx this should be clarified; it doesn't in particular prevent other processes
# from opening the file, at least currently.
when defined(windows): when defined(windows):
let dwShareMode = FILE_SHARE_DELETE or FILE_SHARE_READ or FILE_SHARE_WRITE let dwShareMode = FILE_SHARE_DELETE or FILE_SHARE_READ or FILE_SHARE_WRITE
let dwCreation = CREATE_NEW let dwCreation = CREATE_NEW
@ -64,9 +68,13 @@ proc safeOpen(filename: string): File =
discard close_osfandle(fileHandle) discard close_osfandle(fileHandle)
raiseOSError(osLastError(), filename) raiseOSError(osLastError(), filename)
else: else:
# xxx we need a `proc toMode(a: FilePermission): Mode`, possibly by
# exposing fusion/filepermissions.fromFilePermissions to stdlib; then we need
# to expose a `perm` param so users can customize this (e.g. the temp file may
# need execute permissions), and figure out how to make the API cross platform.
let mode = Mode(S_IRUSR or S_IWUSR)
let flags = posix.O_RDWR or posix.O_CREAT or posix.O_EXCL let flags = posix.O_RDWR or posix.O_CREAT or posix.O_EXCL
let fileHandle = posix.open(filename, flags, mode)
let fileHandle = posix.open(filename, flags)
if fileHandle == -1: if fileHandle == -1:
raiseOSError(osLastError(), filename) raiseOSError(osLastError(), filename)
@ -93,7 +101,6 @@ proc createTempFile*(prefix, suffix: string, dir = ""): tuple[fd: File, path: st
## If failing to create a temporary file, `IOError` will be raised. ## If failing to create a temporary file, `IOError` will be raised.
## ##
## .. note:: It is the caller's responsibility to remove the file when no longer needed. ## .. note:: It is the caller's responsibility to remove the file when no longer needed.
##
var dir = dir var dir = dir
if dir.len == 0: if dir.len == 0:
dir = getTempDir() dir = getTempDir()