Skip to content

Commit

Permalink
Fix null termination (#183)
Browse files Browse the repository at this point in the history
Fixes #181 

In general, we should accept `[]const u8` and return `[:0]const u8`
where possible.

We switch from using PyObject_GetAttrString and equivalents to
PyObject_GetAttr since the former simply creates a PyUnicode object
anyway. By creating the object ourselves, we're able to construct it
from non-null terminated slices without making a second copy.

The actual bug we had, was the `PyModule.from_code` function was using
`[]const u8` slices as `value.ptr` without requirement them to be null
terminated.
  • Loading branch information
gatesn authored Oct 4, 2023
1 parent b7ba227 commit 1576190
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 17 deletions.
5 changes: 1 addition & 4 deletions pydust/src/types/error.zig
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,7 @@ const PyExc = struct {
const fake_module = try py.PyModule.fromCode(code, line_info.file_name, symbol_info.compile_unit_name);
defer fake_module.decref();

const func_name = try py.allocator.dupeZ(u8, symbol_info.symbol_name);
defer py.allocator.free(func_name);

_ = fake_module.obj.call(void, func_name, .{}, .{}) catch null;
_ = fake_module.obj.call(void, symbol_info.symbol_name, .{}, .{}) catch null;

// Grab our forced exception info.
// We can ignore qtype and qvalue, we just want to get the traceback object.
Expand Down
12 changes: 10 additions & 2 deletions pydust/src/types/module.zig
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,18 @@ pub const PyModule = extern struct {

/// Create and insantiate a PyModule object from a Python code string.
pub fn fromCode(code: []const u8, filename: []const u8, module_name: []const u8) !PyModule {
const pycode = ffi.Py_CompileString(code.ptr, filename.ptr, ffi.Py_file_input) orelse return PyError.PyRaised;
// Ensure null-termination of all strings
const codeZ = try py.allocator.dupeZ(u8, code);
defer py.allocator.free(codeZ);
const filenameZ = try py.allocator.dupeZ(u8, filename);
defer py.allocator.free(filenameZ);
const module_nameZ = try py.allocator.dupeZ(u8, module_name);
defer py.allocator.free(module_nameZ);

const pycode = ffi.Py_CompileString(codeZ.ptr, filenameZ.ptr, ffi.Py_file_input) orelse return PyError.PyRaised;
defer ffi.Py_DECREF(pycode);

const pymod = ffi.PyImport_ExecCodeModuleEx(module_name.ptr, pycode, filename.ptr) orelse return PyError.PyRaised;
const pymod = ffi.PyImport_ExecCodeModuleEx(module_nameZ.ptr, pycode, filenameZ.ptr) orelse return PyError.PyRaised;
return .{ .obj = .{ .py = pymod } };
}
};
30 changes: 19 additions & 11 deletions pydust/src/types/obj.zig
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,23 @@ pub const PyObject = extern struct {
}

/// Call a method on this object with no arguments.
pub fn call0(self: PyObject, comptime T: type, method: [:0]const u8) !T {
pub fn call0(self: PyObject, comptime T: type, method: []const u8) !T {
return py.call0(T, try self.get(method));
}

/// Call a method on this object with the given args and kwargs.
pub fn call(self: PyObject, comptime T: type, method: [:0]const u8, args: anytype, kwargs: anytype) !T {
const methodObj = try self.get(method);
return py.call(T, methodObj, args, kwargs);
pub fn call(self: PyObject, comptime T: type, method: []const u8, args: anytype, kwargs: anytype) !T {
return py.call(T, try self.get(method), args, kwargs);
}

pub fn get(self: PyObject, attr: [:0]const u8) !py.PyObject {
return .{ .py = ffi.PyObject_GetAttrString(self.py, attr) orelse return PyError.PyRaised };
pub fn get(self: PyObject, attr: []const u8) !py.PyObject {
const attrStr = try py.PyString.create(attr);
defer attrStr.decref();

return .{ .py = ffi.PyObject_GetAttr(self.py, attrStr.obj.py) orelse return PyError.PyRaised };
}

pub fn getAs(self: PyObject, comptime T: type, attr: [:0]const u8) !T {
pub fn getAs(self: PyObject, comptime T: type, attr: []const u8) !T {
return try py.as(T, try self.get(attr));
}

Expand All @@ -65,15 +67,21 @@ pub const PyObject = extern struct {
return buffer;
}

pub fn set(self: PyObject, attr: [:0]const u8, value: PyObject) !PyObject {
if (ffi.PyObject_SetAttrString(self.py, attr, value.py) < 0) {
pub fn set(self: PyObject, attr: []const u8, value: PyObject) !PyObject {
const attrStr = try py.PyString.create(attr);
defer attrStr.decref();

if (ffi.PyObject_SetAttr(self.py, attrStr.obj.py, value.py) < 0) {
return PyError.PyRaised;
}
return self;
}

pub fn del(self: PyObject, attr: [:0]const u8) !PyObject {
if (ffi.PyObject_DelAttrString(self.py, attr) < 0) {
pub fn del(self: PyObject, attr: []const u8) !PyObject {
const attrStr = try py.PyString.create(attr);
defer attrStr.decref();

if (ffi.PyObject_DelAttr(self.py, attrStr.obj.py) < 0) {
return PyError.PyRaised;
}
return self;
Expand Down

0 comments on commit 1576190

Please sign in to comment.