Skip to content

Commit 6b6b949

Browse files
committed
Allow fcntl functions to accept objects with fileno() function
As specified in Python official documentation (https://docs.python.org/3.10/library/fcntl.html) > All functions in this module take a file descriptor fd as their first > argument. This can be an integer file descriptor, such as returned by > sys.stdin.fileno(), or an io.IOBase object, such as sys.stdin itself, > which provides a fileno() that returns a genuine file descriptor. And clarified more in fcntl.fcntl function: > Perform the operation cmd on file descriptor fd (file objects > providing a fileno() method are accepted as well). All function in fcntl modules should accept either int fd or object with fileno() function which returns int fd. This was already implemented with for `fcntl.fcntl` function with `io::Fildes` newtype helper which extracted either int fd or called fileno() function, and it was also implemented with duplicated ad-hoc code for `fcntl.ioctl`. This commit replaces the ad-hoc implementation with `io::Fildes` and adds it to missing functions: `fcntl.flock` and `fcntl.lockf`. For more information that this is implemented in the same way as CPython, you can check the corresponding CPython module: https://github.com/python/cpython/blob/3.10/Modules/clinic/fcntlmodule.c.h The functions are: `fcntl_fcntl`, `fcntl_ioctl`, `fcntl_flock` and `fcntl_lockf`, all of these functions use `_PyLong_FileDescriptor_Converter` which does the same thing as RustPython's `io::Fildes`, meaning it either extracts the argument as int fd or calls fileno() function on passed object that returns the int fd. Here is the implementation for `_PyLong_FileDescriptor_Converter`: https://github.com/python/cpython/blob/3.10/Objects/fileobject.c#L227 which in turns calls `PyObject_AsFileDescriptor` which is located here https://github.com/python/cpython/blob/3.10/Objects/fileobject.c#L180 in the same file and we can see that it tries to convert it to int or call fileno() function. Note regarding the python unit tests `test_flock`, `test_lockf_exclusive`, `test_lockf_shared`: The tests no longer fail with `TypeError: 'BufferedRandom' object cannot be interpreted as an integer` which makes `test_flock` pass the test, but `test_lockf_exclusive` and `test_lockf_exclusive` still fail but not on fcntl calls, they fail on `Process()` constructor with `AttributeError: module 'os' has no attribute 'fork'` which seems unrelated with this change. This unrelated error was probably never detected as fcntl calls failed earlier, before `Process()` could even be called.
1 parent 8001ff8 commit 6b6b949

File tree

2 files changed

+5
-17
lines changed

2 files changed

+5
-17
lines changed

Lib/test/test_fcntl.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,6 @@ def test_fcntl_64_bit(self):
141141
finally:
142142
os.close(fd)
143143

144-
# TODO: RUSTPYTHON, TypeError: 'BufferedRandom' object cannot be interpreted as an integer
145-
@unittest.expectedFailure
146144
def test_flock(self):
147145
# Solaris needs readable file for shared lock
148146
self.f = open(TESTFN, 'wb+')
@@ -157,7 +155,7 @@ def test_flock(self):
157155
self.assertRaises(ValueError, fcntl.flock, -1, fcntl.LOCK_SH)
158156
self.assertRaises(TypeError, fcntl.flock, 'spam', fcntl.LOCK_SH)
159157

160-
# TODO: RUSTPYTHON, TypeError: 'BufferedRandom' object cannot be interpreted as an integer
158+
# TODO: RUSTPYTHON, AttributeError: module 'os' has no attribute 'fork'
161159
@unittest.expectedFailure
162160
@unittest.skipIf(platform.system() == "AIX", "AIX returns PermissionError")
163161
def test_lockf_exclusive(self):
@@ -170,7 +168,7 @@ def test_lockf_exclusive(self):
170168
fcntl.lockf(self.f, fcntl.LOCK_UN)
171169
self.assertEqual(p.exitcode, 0)
172170

173-
# TODO: RUSTPYTHON, TypeError: 'BufferedRandom' object cannot be interpreted as an integer
171+
# TODO: RUSTPYTHON, AttributeError: module 'os' has no attribute 'fork'
174172
@unittest.expectedFailure
175173
@unittest.skipIf(platform.system() == "AIX", "AIX returns PermissionError")
176174
def test_lockf_share(self):

stdlib/src/fcntl.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ mod fcntl {
5252
#[cfg(any(target_os = "dragonfly", target_os = "netbsd", target_vendor = "apple"))]
5353
#[pyattr]
5454
use libc::F_GETPATH;
55-
use rustpython_vm::PyObjectRef;
5655

5756
#[pyfunction]
5857
fn fcntl(
@@ -90,21 +89,12 @@ mod fcntl {
9089

9190
#[pyfunction]
9291
fn ioctl(
93-
obj: PyObjectRef,
92+
io::Fildes(fd): io::Fildes,
9493
request: u32,
9594
arg: OptionalArg<Either<Either<ArgMemoryBuffer, ArgStrOrBytesLike>, i32>>,
9695
mutate_flag: OptionalArg<bool>,
9796
vm: &VirtualMachine,
9897
) -> PyResult {
99-
let fd = obj.try_to_value(vm).or_else(|_| {
100-
let meth = vm.get_method_or_type_error(
101-
obj.clone(),
102-
vm.ctx.interned_str("fileno").unwrap(),
103-
|| "ioctl first arg must be an int or object with a fileno() method".to_owned(),
104-
)?;
105-
vm.invoke(&meth, ())?.try_into_value(vm)
106-
})?;
107-
10898
let arg = arg.unwrap_or_else(|| Either::B(0));
10999
match arg {
110100
Either::A(buf_kind) => {
@@ -153,7 +143,7 @@ mod fcntl {
153143
// XXX: at the time of writing, wasi and redox don't have the necessary constants/function
154144
#[cfg(not(any(target_os = "wasi", target_os = "redox")))]
155145
#[pyfunction]
156-
fn flock(fd: i32, operation: i32, vm: &VirtualMachine) -> PyResult {
146+
fn flock(io::Fildes(fd): io::Fildes, operation: i32, vm: &VirtualMachine) -> PyResult {
157147
let ret = unsafe { libc::flock(fd, operation) };
158148
// TODO: add support for platforms that don't have a builtin `flock` syscall
159149
if ret < 0 {
@@ -166,7 +156,7 @@ mod fcntl {
166156
#[cfg(not(any(target_os = "wasi", target_os = "redox")))]
167157
#[pyfunction]
168158
fn lockf(
169-
fd: i32,
159+
io::Fildes(fd): io::Fildes,
170160
cmd: i32,
171161
len: OptionalArg<PyIntRef>,
172162
start: OptionalArg<PyIntRef>,

0 commit comments

Comments
 (0)