Skip to content

Commit 37912df

Browse files
committed
Add closefd arg to open
1 parent 10c63da commit 37912df

File tree

3 files changed

+98
-22
lines changed

3 files changed

+98
-22
lines changed

vm/src/function.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,10 @@ pub trait FromArgs: Sized {
261261
pub struct KwArgs<T = PyObjectRef>(HashMap<String, T>);
262262

263263
impl<T> KwArgs<T> {
264+
pub fn new(map: HashMap<String, T>) -> Self {
265+
KwArgs(map)
266+
}
267+
264268
pub fn pop_kwarg(&mut self, name: &str) -> Option<T> {
265269
self.0.remove(name)
266270
}

vm/src/stdlib/io.rs

Lines changed: 93 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::io::{self, prelude::*, Cursor, SeekFrom};
88
use num_traits::ToPrimitive;
99

1010
use crate::exceptions::PyBaseExceptionRef;
11-
use crate::function::{OptionalArg, OptionalOption, PyFuncArgs};
11+
use crate::function::{Args, KwArgs, OptionalArg, OptionalOption, PyFuncArgs};
1212
use crate::obj::objbool;
1313
use crate::obj::objbytearray::PyByteArray;
1414
use crate::obj::objbyteinner::PyBytesLike;
@@ -577,35 +577,57 @@ mod fileio {
577577
flag as u32
578578
}
579579

580-
fn file_io_init(
581-
file_io: PyObjectRef,
580+
#[derive(FromArgs)]
581+
struct FileIOArgs {
582+
#[pyarg(positional_only)]
582583
name: Either<PyStringRef, i64>,
583-
mode: OptionalArg<PyStringRef>,
584-
vm: &VirtualMachine,
585-
) -> PyResult {
586-
let (name, file_no) = match name {
584+
#[pyarg(positional_or_keyword, default = "None")]
585+
mode: Option<PyStringRef>,
586+
#[pyarg(positional_or_keyword, default = "true")]
587+
closefd: bool,
588+
#[pyarg(positional_or_keyword, default = "None")]
589+
opener: Option<PyObjectRef>,
590+
}
591+
fn file_io_init(file_io: PyObjectRef, args: FileIOArgs, vm: &VirtualMachine) -> PyResult {
592+
let (name, file_no) = match args.name {
587593
Either::A(name) => {
588-
let mode = match mode {
589-
OptionalArg::Present(mode) => compute_c_flag(mode.as_str()),
590-
OptionalArg::Missing => libc::O_RDONLY as _,
594+
if !args.closefd {
595+
return Err(
596+
vm.new_value_error("Cannot use closefd=False with file name".to_owned())
597+
);
598+
}
599+
let mode = match args.mode {
600+
Some(mode) => compute_c_flag(mode.as_str()),
601+
None => libc::O_RDONLY as _,
591602
};
592-
(
593-
name.clone().into_object(),
603+
let fd = if let Some(opener) = args.opener {
604+
let fd =
605+
vm.invoke(&opener, vec![name.clone().into_object(), vm.new_int(mode)])?;
606+
if !vm.isinstance(&fd, &vm.ctx.types.int_type)? {
607+
return Err(vm.new_type_error("expected integer from opener".to_owned()));
608+
}
609+
let fd = i64::try_from_object(vm, fd)?;
610+
if fd < 0 {
611+
return Err(vm.new_os_error("Negative file descriptor".to_owned()));
612+
}
613+
fd
614+
} else {
594615
os::os_open(
595616
os::PyPathLike::new_str(name.as_str().to_owned()),
596617
mode as _,
597618
OptionalArg::Missing,
598619
OptionalArg::Missing,
599620
vm,
600-
)?,
601-
)
621+
)?
622+
};
623+
(name.clone().into_object(), fd)
602624
}
603625
Either::B(fno) => (vm.new_int(fno), fno),
604626
};
605627

606628
vm.set_attr(&file_io, "name", name)?;
607629
vm.set_attr(&file_io, "__fileno", vm.new_int(file_no))?;
608-
vm.set_attr(&file_io, "closefd", vm.new_bool(false))?;
630+
vm.set_attr(&file_io, "__closefd", vm.new_bool(args.closefd))?;
609631
vm.set_attr(&file_io, "__closed", vm.new_bool(false))?;
610632
Ok(vm.get_none())
611633
}
@@ -701,9 +723,12 @@ mod fileio {
701723
}
702724

703725
fn file_io_close(instance: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
704-
let raw_handle = i64::try_from_object(vm, vm.get_attribute(instance.clone(), "__fileno")?)?;
705-
drop(os::rust_file(raw_handle));
706-
vm.set_attr(&instance, "closefd", vm.new_bool(true))?;
726+
let closefd = objbool::boolval(vm, vm.get_attribute(instance.clone(), "__closefd")?)?;
727+
if closefd {
728+
let raw_handle =
729+
i64::try_from_object(vm, vm.get_attribute(instance.clone(), "__fileno")?)?;
730+
drop(os::rust_file(raw_handle));
731+
}
707732
vm.set_attr(&instance, "__closed", vm.new_bool(true))?;
708733
Ok(())
709734
}
@@ -949,16 +974,56 @@ fn split_mode_string(mode_string: &str) -> Result<(String, String), String> {
949974
fn io_open_wrapper(
950975
file: PyObjectRef,
951976
mode: OptionalArg<PyStringRef>,
977+
opts: OpenArgs,
952978
vm: &VirtualMachine,
953979
) -> PyResult {
954-
io_open(file, mode.as_ref().into_option().map(|s| s.as_str()), vm)
980+
io_open(
981+
file,
982+
mode.as_ref().into_option().map(|s| s.as_str()),
983+
opts,
984+
vm,
985+
)
955986
}
956987
fn io_open_code(file: PyObjectRef, vm: &VirtualMachine) -> PyResult {
957988
// TODO: lifecycle hooks or something?
958-
io_open(file, Some("rb"), vm)
989+
io_open(file, Some("rb"), Default::default(), vm)
990+
}
991+
992+
#[derive(FromArgs)]
993+
#[allow(unused)]
994+
pub struct OpenArgs {
995+
#[pyarg(positional_or_keyword, default = "-1")]
996+
buffering: isize,
997+
#[pyarg(positional_or_keyword, default = "None")]
998+
encoding: Option<PyStringRef>,
999+
#[pyarg(positional_or_keyword, default = "None")]
1000+
errors: Option<PyStringRef>,
1001+
#[pyarg(positional_or_keyword, default = "None")]
1002+
newline: Option<PyStringRef>,
1003+
#[pyarg(positional_or_keyword, default = "true")]
1004+
closefd: bool,
1005+
#[pyarg(positional_or_keyword, default = "None")]
1006+
opener: Option<PyObjectRef>,
1007+
}
1008+
impl Default for OpenArgs {
1009+
fn default() -> Self {
1010+
OpenArgs {
1011+
buffering: -1,
1012+
encoding: None,
1013+
errors: None,
1014+
newline: None,
1015+
closefd: true,
1016+
opener: None,
1017+
}
1018+
}
9591019
}
9601020

961-
pub fn io_open(file: PyObjectRef, mode: Option<&str>, vm: &VirtualMachine) -> PyResult {
1021+
pub fn io_open(
1022+
file: PyObjectRef,
1023+
mode: Option<&str>,
1024+
opts: OpenArgs,
1025+
vm: &VirtualMachine,
1026+
) -> PyResult {
9621027
// mode is optional: 'rt' is the default mode (open from reading text)
9631028
let mode_string = mode.unwrap_or("rt");
9641029

@@ -981,7 +1046,13 @@ pub fn io_open(file: PyObjectRef, mode: Option<&str>, vm: &VirtualMachine) -> Py
9811046
})?;
9821047
let file_io_obj = vm.invoke(
9831048
&file_io_class,
984-
vec![file.clone(), vm.ctx.new_str(mode.clone())],
1049+
PyFuncArgs::from((
1050+
Args::new(vec![file.clone(), vm.ctx.new_str(mode.clone())]),
1051+
KwArgs::new(maplit::hashmap! {
1052+
"closefd".to_owned() => vm.new_bool(opts.closefd),
1053+
"opener".to_owned() => opts.opener.unwrap_or_else(|| vm.get_none()),
1054+
}),
1055+
)),
9851056
)?;
9861057

9871058
// Create Buffered class to consume FileIO. The type of buffered class depends on

vm/src/stdlib/subprocess.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ fn convert_to_file_io(file: &Option<File>, mode: &str, vm: &VirtualMachine) -> P
9595
Some(ref stdin) => io_open(
9696
vm.new_int(raw_file_number(stdin.try_clone().unwrap())),
9797
Some(mode),
98+
Default::default(),
9899
vm,
99100
),
100101
None => Ok(vm.get_none()),

0 commit comments

Comments
 (0)