Skip to content
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 51 additions & 7 deletions rclpy/src/rclpy/_rclpy.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,58 @@ rclpy_trigger_guard_condition(PyObject * Py_UNUSED(self), PyObject * args)
* Raises RuntimeError if rcl could not be initialized
*/
static PyObject *
rclpy_init(PyObject * Py_UNUSED(self), PyObject * Py_UNUSED(args))
rclpy_init(PyObject * Py_UNUSED(self), PyObject * args)
{
// TODO(esteve): parse args
rcl_ret_t ret = rcl_init(0, NULL, rcl_get_default_allocator());
if (ret != RCL_RET_OK) {
PyErr_Format(PyExc_RuntimeError,
"Failed to init: %s", rcl_get_error_string_safe());
rcl_reset_error();
// Expect one argument which is a list of strings
PyObject * pyargs;
if (!PyArg_ParseTuple(args, "O", &pyargs)) {
// Exception raised
return NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check, these functions setup the exceptions right? That's why only returning null here is fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, PyArg_ParseTuple returns false and sets an exception on failure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments about exceptions already being set in 023ccaf

}

pyargs = PySequence_List(pyargs);
if (NULL == pyargs) {
// Exception raised
return NULL;
}
Py_ssize_t pysize_num_args = PyList_Size(pyargs);
if (pysize_num_args > INT_MAX) {
PyErr_Format(PyExc_OverflowError, "Too many arguments");
return NULL;
}
int num_args = (int)pysize_num_args;

rcl_allocator_t allocator = rcl_get_default_allocator();
char ** arg_values = allocator.allocate(sizeof(char *) * num_args, allocator.state);
if (NULL == arg_values) {
PyErr_Format(PyExc_MemoryError, "Failed to allocate space for arguments");
Py_DECREF(pyargs);
return NULL;
}

bool have_args = true;
for (int i = 0; i < num_args; ++i) {
// Returns borrowed reference, do not decref
PyObject * pyarg = PyList_GetItem(pyargs, i);
if (NULL == pyarg) {
have_args = false;
break;
}
// Borrows a pointer, do not free arg_values[i]
arg_values[i] = PyUnicode_AsUTF8(pyarg);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just skip this entire block (L180 to 197) when num_args is 0 and pass a NULL pointer to rcl_init instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block skipped when no args in 93c634b


if (have_args) {
rcl_ret_t ret = rcl_init(num_args, arg_values, allocator);
if (ret != RCL_RET_OK) {
PyErr_Format(PyExc_RuntimeError, "Failed to init: %s", rcl_get_error_string_safe());
rcl_reset_error();
}
}
allocator.deallocate(arg_values, allocator.state);
Py_DECREF(pyargs);

if (PyErr_Occurred()) {
return NULL;
}
Py_RETURN_NONE;
Expand Down