Mercurial > hg
changeset 43208:1ca3823aeefd
rust-cpython: add wrapper around decapsule_make_dirstate_tuple()
There are a couple of safety issues. First, the returned function pointer
must be unsafe. Second, its return value must be a raw pointer (i.e.
python27_sys::PyObject), not a cpython::PyObject. The wrapper function will
address these issues.
| author | Yuya Nishihara <yuya@tcha.org> |
|---|---|
| date | Sun, 13 Oct 2019 16:55:17 +0900 |
| parents | 4aa9f3a1c1df |
| children | 8b355cbef16d |
| files | rust/hg-cpython/src/dirstate.rs rust/hg-cpython/src/dirstate/dirstate_map.rs rust/hg-cpython/src/parsers.rs |
| diffstat | 3 files changed, 30 insertions(+), 71 deletions(-) [+] |
line wrap: on
line diff
--- a/rust/hg-cpython/src/dirstate.rs Sun Oct 13 02:10:26 2019 +0200 +++ b/rust/hg-cpython/src/dirstate.rs Sun Oct 13 16:55:17 2019 +0900 @@ -46,9 +46,7 @@ /// This is largely a copy/paste from cindex.rs, pending the merge of a /// `py_capsule_fn!` macro in the rust-cpython project: /// https://github.com/dgrunwald/rust-cpython/pull/169 -pub fn decapsule_make_dirstate_tuple( - py: Python, -) -> PyResult<MakeDirstateTupleFn> { +fn decapsule_make_dirstate_tuple(py: Python) -> PyResult<MakeDirstateTupleFn> { unsafe { let caps_name = CStr::from_bytes_with_nul_unchecked( b"mercurial.cext.parsers.make_dirstate_tuple_CAPI\0", @@ -61,6 +59,25 @@ } } +pub fn make_dirstate_tuple( + py: Python, + entry: &DirstateEntry, +) -> PyResult<PyObject> { + let make = decapsule_make_dirstate_tuple(py)?; + + let &DirstateEntry { + state, + mode, + size, + mtime, + } = entry; + // Explicitly go through u8 first, then cast to platform-specific `c_char` + // because Into<u8> has a specific implementation while `as c_char` would + // just do a naive enum cast. + let state_code: u8 = state.into(); + Ok(make(state_code as c_char, mode, size, mtime)) +} + pub fn extract_dirstate(py: Python, dmap: &PyDict) -> Result<StateMap, PyErr> { dmap.items(py) .iter()
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs Sun Oct 13 02:10:26 2019 +0200 +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs Sun Oct 13 16:55:17 2019 +0900 @@ -16,11 +16,10 @@ exc, ObjectProtocol, PyBool, PyBytes, PyClone, PyDict, PyErr, PyObject, PyResult, PyTuple, Python, PythonObject, ToPyObject, }; -use libc::c_char; use crate::{ dirstate::copymap::{CopyMap, CopyMapItemsIterator, CopyMapKeysIterator}, - dirstate::{decapsule_make_dirstate_tuple, dirs_multiset::Dirs}, + dirstate::{dirs_multiset::Dirs, make_dirstate_tuple}, ref_sharing::{PyLeakedRef, PySharedRefCell}, }; use hg::{ @@ -66,15 +65,7 @@ let key = key.extract::<PyBytes>(py)?; match self.inner(py).borrow().get(HgPath::new(key.data(py))) { Some(entry) => { - // Explicitly go through u8 first, then cast to - // platform-specific `c_char`. - let state: u8 = entry.state.into(); - Ok(Some(decapsule_make_dirstate_tuple(py)?( - state as c_char, - entry.mode, - entry.size, - entry.mtime, - ))) + Ok(Some(make_dirstate_tuple(py, entry)?)) }, None => Ok(default) } @@ -303,15 +294,7 @@ let key = HgPath::new(key.data(py)); match self.inner(py).borrow().get(key) { Some(entry) => { - // Explicitly go through u8 first, then cast to - // platform-specific `c_char`. - let state: u8 = entry.state.into(); - Ok(decapsule_make_dirstate_tuple(py)?( - state as c_char, - entry.mode, - entry.size, - entry.mtime, - )) + Ok(make_dirstate_tuple(py, entry)?) }, None => Err(PyErr::new::<exc::KeyError, _>( py, @@ -493,18 +476,9 @@ res: (&HgPathBuf, &DirstateEntry), ) -> PyResult<Option<(PyBytes, PyObject)>> { let (f, entry) = res; - - // Explicitly go through u8 first, then cast to - // platform-specific `c_char`. - let state: u8 = entry.state.into(); Ok(Some(( PyBytes::new(py, f.as_ref()), - decapsule_make_dirstate_tuple(py)?( - state as c_char, - entry.mode, - entry.size, - entry.mtime, - ), + make_dirstate_tuple(py, entry)?, ))) } }
--- a/rust/hg-cpython/src/parsers.rs Sun Oct 13 02:10:26 2019 +0200 +++ b/rust/hg-cpython/src/parsers.rs Sun Oct 13 16:55:17 2019 +0900 @@ -15,15 +15,13 @@ PythonObject, ToPyObject, }; use hg::{ - pack_dirstate, parse_dirstate, utils::hg_path::HgPathBuf, DirstateEntry, + pack_dirstate, parse_dirstate, utils::hg_path::HgPathBuf, DirstatePackError, DirstateParents, DirstateParseError, PARENT_SIZE, }; use std::collections::HashMap; use std::convert::TryInto; -use libc::c_char; - -use crate::dirstate::{decapsule_make_dirstate_tuple, extract_dirstate}; +use crate::dirstate::{extract_dirstate, make_dirstate_tuple}; use std::time::Duration; fn parse_dirstate_wrapper( @@ -37,22 +35,11 @@ match parse_dirstate(&mut dirstate_map, &mut copies, st.data(py)) { Ok(parents) => { - for (filename, entry) in dirstate_map { - // Explicitly go through u8 first, then cast to - // platform-specific `c_char` because Into<u8> has a specific - // implementation while `as c_char` would just do a naive enum - // cast. - let state: u8 = entry.state.into(); - + for (filename, entry) in &dirstate_map { dmap.set_item( py, PyBytes::new(py, filename.as_ref()), - decapsule_make_dirstate_tuple(py)?( - state as c_char, - entry.mode, - entry.size, - entry.mtime, - ), + make_dirstate_tuple(py, entry)?, )?; } for (path, copy_path) in copies { @@ -127,30 +114,11 @@ Duration::from_secs(now.as_object().extract::<u64>(py)?), ) { Ok(packed) => { - for ( - filename, - DirstateEntry { - state, - mode, - size, - mtime, - }, - ) in dirstate_map - { - // Explicitly go through u8 first, then cast to - // platform-specific `c_char` because Into<u8> has a specific - // implementation while `as c_char` would just do a naive enum - // cast. - let state: u8 = state.into(); + for (filename, entry) in &dirstate_map { dmap.set_item( py, PyBytes::new(py, filename.as_ref()), - decapsule_make_dirstate_tuple(py)?( - state as c_char, - mode, - size, - mtime, - ), + make_dirstate_tuple(py, entry)?, )?; } Ok(PyBytes::new(py, &packed))