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.
--- 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))