Skip to content

Commit

Permalink
Merge pull request #1133 from birkenfeld/string-apis
Browse files Browse the repository at this point in the history
Avoid using CString where unnecessary
  • Loading branch information
kngwyu authored Sep 8, 2020
2 parents 16fe583 + 466ffea commit b2ba83a
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/ffi/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub unsafe fn PyImport_ImportModuleEx(

extern "C" {
pub fn PyImport_GetImporter(path: *mut PyObject) -> *mut PyObject;
#[cfg_attr(PyPy, link_name = "PyPyImport_Import")]
pub fn PyImport_Import(name: *mut PyObject) -> *mut PyObject;
#[cfg_attr(PyPy, link_name = "PyPyImport_ReloadModule")]
pub fn PyImport_ReloadModule(m: *mut PyObject) -> *mut PyObject;
Expand Down
15 changes: 7 additions & 8 deletions src/type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,17 +229,16 @@ fn initialize_tp_dict(
tp_dict: *mut ffi::PyObject,
items: Vec<(&'static str, PyObject)>,
) -> PyResult<()> {
use std::ffi::CString;

// We hold the GIL: the dictionary update can be considered atomic from
// the POV of other threads.
for (key, val) in items {
let ret = unsafe {
ffi::PyDict_SetItemString(tp_dict, CString::new(key)?.as_ptr(), val.into_ptr())
};
if ret < 0 {
return Err(PyErr::fetch(py));
}
crate::types::with_tmp_string(py, key, |key| {
let ret = unsafe { ffi::PyDict_SetItem(tp_dict, key, val.into_ptr()) };
if ret < 0 {
return Err(PyErr::fetch(py));
}
Ok(())
})?;
}
Ok(())
}
Expand Down
1 change: 1 addition & 0 deletions src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub use self::num::PyLong as PyInt;
pub use self::sequence::PySequence;
pub use self::set::{PyFrozenSet, PySet};
pub use self::slice::{PySlice, PySliceIndices};
pub(crate) use self::string::with_tmp_string;
pub use self::string::{PyString, PyString as PyUnicode};
pub use self::tuple::PyTuple;
pub use self::typeobject::PyType;
Expand Down
6 changes: 4 additions & 2 deletions src/types/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,16 @@ pyobject_native_var_type!(PyModule, ffi::PyModule_Type, ffi::PyModule_Check);
impl PyModule {
/// Creates a new module object with the `__name__` attribute set to name.
pub fn new<'p>(py: Python<'p>, name: &str) -> PyResult<&'p PyModule> {
// Could use PyModule_NewObject, but it doesn't exist on PyPy.
let name = CString::new(name)?;
unsafe { py.from_owned_ptr_or_err(ffi::PyModule_New(name.as_ptr())) }
}

/// Imports the Python module with the specified name.
pub fn import<'p>(py: Python<'p>, name: &str) -> PyResult<&'p PyModule> {
let name = CString::new(name)?;
unsafe { py.from_owned_ptr_or_err(ffi::PyImport_ImportModule(name.as_ptr())) }
crate::types::with_tmp_string(py, name, |name| unsafe {
py.from_owned_ptr_or_err(ffi::PyImport_Import(name))
})
}

/// Loads the Python code specified into a new module.
Expand Down
18 changes: 18 additions & 0 deletions src/types/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,24 @@ impl PyString {
}
}

/// Convenience for calling Python APIs with a temporary string
/// object created from a given Rust string.
pub(crate) fn with_tmp_string<F, R>(py: Python, s: &str, f: F) -> PyResult<R>
where
F: FnOnce(*mut ffi::PyObject) -> PyResult<R>,
{
unsafe {
let s_obj =
ffi::PyUnicode_FromStringAndSize(s.as_ptr() as *const _, s.len() as ffi::Py_ssize_t);
if s_obj.is_null() {
return Err(PyErr::fetch(py));
}
let ret = f(s_obj);
ffi::Py_DECREF(s_obj);
ret
}
}

/// Converts a Rust `str` to a Python object.
/// See `PyString::new` for details on the conversion.
impl ToPyObject for str {
Expand Down

0 comments on commit b2ba83a

Please sign in to comment.