Skip to content

Commit c35ff3a

Browse files
authored
Align mmap read and search arguments (#9590)
* Preserve mmap read index conversion errors Distinguish a missing index slot from exceptions raised while converting an index, including overflow and interrupts. Assisted-by: OpenAI Codex:model version not exposed * Align mmap search argument handling Assisted-by: OpenAI Codex:model version not exposed * Reuse UTF-8 boundary truncation in mmap read errors Apply maintainer review while preserving conversion and audit behavior. Assisted-by: OpenAI Codex:model version unavailable * Compute omitted mmap search bound lazily Use map_or_else for the pure default bound calculation to satisfy strict Clippy. Assisted-by: OpenAI Codex:model version unavailable
1 parent 1dbe198 commit c35ff3a

1 file changed

Lines changed: 59 additions & 23 deletions

File tree

‎crates/stdlib/src/mmap.rs‎

Lines changed: 59 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -343,14 +343,14 @@ mod mmap {
343343
}
344344
}
345345

346-
#[derive(FromArgs, Clone)]
346+
#[derive(FromArgs)]
347347
pub(super) struct FindOptions {
348348
#[pyarg(positional)]
349-
view: Vec<u8>,
349+
view: PyObjectRef,
350350
#[pyarg(positional, optional)]
351-
start: Option<isize>,
351+
start: Option<PyObjectRef>,
352352
#[pyarg(positional, optional)]
353-
end: Option<isize>,
353+
end: Option<PyObjectRef>,
354354
}
355355

356356
#[cfg(all(unix, not(target_os = "redox")))]
@@ -894,10 +894,16 @@ mod mmap {
894894
Ok(())
895895
}
896896

897-
fn get_find_range(&self, options: &FindOptions) -> (usize, usize) {
897+
fn get_find_range(
898+
&self,
899+
initial_pos: usize,
900+
initial_size: usize,
901+
start: Option<isize>,
902+
end: Option<isize>,
903+
) -> (usize, usize) {
898904
let size = self.__len__();
899-
let start = options.start.map_or_else(
900-
|| self.pos(),
905+
let start = start.map_or_else(
906+
|| initial_pos,
901907
|start| {
902908
if start < 0 {
903909
start.saturated_at(size)
@@ -906,7 +912,7 @@ mod mmap {
906912
}
907913
},
908914
);
909-
let end = options.end.map_or(size, |end| end.saturated_at(size));
915+
let end = end.map_or_else(|| initial_size.min(size), |end| end.saturated_at(size));
910916
(start, end)
911917
}
912918

@@ -916,15 +922,44 @@ mod mmap {
916922
reverse: bool,
917923
vm: &VirtualMachine,
918924
) -> PyResult<PyInt> {
925+
// Bind the positional arguments before acquiring the buffer, as
926+
// Clinic does. The buffer can call Python and close the mapping.
927+
let view = ArgBytesLike::try_from_borrowed_object(vm, &options.view)?;
928+
let mmap = self.check_valid(vm)?;
929+
let initial_pos = self.pos();
930+
let initial_size = self.__len__();
931+
drop(mmap);
932+
933+
let start = options
934+
.start
935+
.as_ref()
936+
.map(|value| isize::try_from_borrowed_object(vm, value))
937+
.transpose()?;
938+
let end = if start.is_some() {
939+
options
940+
.end
941+
.as_ref()
942+
.map(|value| isize::try_from_borrowed_object(vm, value))
943+
.transpose()?
944+
} else {
945+
None
946+
};
947+
// Index conversion may have closed the mapping. Do not borrow
948+
// search bytes from an exporter before reporting that closure.
949+
if self.closed.load() {
950+
return Err(vm.new_value_error("mmap closed or invalid"));
951+
}
952+
// A search view may itself be this mmap (or a view of it). Take
953+
// an unlocked snapshot before locking the mapping for the scan.
954+
let sub = view.borrow_buf_unlocked(vm)?;
919955
let mmap = self.check_valid(vm)?;
920-
let (start, end) = self.get_find_range(&options);
956+
let (start, end) = self.get_find_range(initial_pos, initial_size, start, end);
921957

922958
// A reversed range contains nothing, not even the empty subsequence.
923959
if start > end {
924960
return Ok(PyInt::from(-1isize));
925961
}
926962

927-
let sub = &options.view;
928963
// The empty subsequence matches at the edge the scan begins from:
929964
// the start of the range going forward, the end going backward.
930965
if sub.is_empty() {
@@ -935,9 +970,9 @@ mod mmap {
935970
let buf = &mmap.as_ref().unwrap().as_slice()[start..end];
936971
let mut windows = buf.windows(sub.len());
937972
let pos = if reverse {
938-
windows.rposition(|window| window == sub)
973+
windows.rposition(|window| window == &*sub)
939974
} else {
940-
windows.position(|window| window == sub)
975+
windows.position(|window| window == &*sub)
941976
};
942977

943978
Ok(pos.map_or_else(|| PyInt::from(-1isize), |i| PyInt::from(start + i)))
@@ -1087,19 +1122,20 @@ mod mmap {
10871122

10881123
#[pymethod]
10891124
fn read(zelf: &Py<Self>, args: ReadArgs, vm: &VirtualMachine) -> PyResult<PyBytesRef> {
1090-
let num_bytes = args
1091-
.n
1092-
.map(|obj| {
1093-
let class = obj.class().to_owned();
1094-
obj.try_into_value::<Option<isize>>(vm).map_err(|_| {
1125+
let num_bytes = match args.n {
1126+
Some(obj) if !vm.is_none(&obj) => {
1127+
let index = obj.try_index_opt(vm).ok_or_else(|| {
1128+
let class = obj.class();
1129+
let name = class.slot_name();
1130+
let name = &name[..name.floor_char_boundary(200)];
10951131
vm.new_type_error(format!(
1096-
"read argument must be int or None, not {}",
1097-
class.name()
1132+
"argument should be integer or None, not '{name}'"
10981133
))
1099-
})
1100-
})
1101-
.transpose()?
1102-
.flatten();
1134+
})??;
1135+
Some(index.try_to_primitive::<isize>(vm)?)
1136+
}
1137+
_ => None,
1138+
};
11031139
let mmap = zelf.check_valid(vm)?;
11041140
let pos = zelf.pos();
11051141
let remaining = zelf.__len__().saturating_sub(pos);

0 commit comments

Comments
 (0)