From d5ede9ce80d22e42e5af28dbb10de0fafb5acde1 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Mon, 5 Mar 2018 14:05:46 -0800 Subject: [PATCH 1/4] rclpy_init() passes command line arguments to rcl_init() --- rclpy/src/rclpy/_rclpy.c | 51 ++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/rclpy/src/rclpy/_rclpy.c b/rclpy/src/rclpy/_rclpy.c index 80f87308e..64361d170 100644 --- a/rclpy/src/rclpy/_rclpy.c +++ b/rclpy/src/rclpy/_rclpy.c @@ -155,14 +155,51 @@ 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)) { + return NULL; + } + + pyargs = PySequence_List(pyargs); + if (NULL == pyargs) { + return NULL; + } + Py_ssize_t num_args = PyList_Size(pyargs); + + 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); + } + + 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; From 023ccafde1fdf0026b47f19a06d978f19f946dcc Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Tue, 6 Mar 2018 08:39:41 -0800 Subject: [PATCH 2/4] comment that exception is already raised --- rclpy/src/rclpy/_rclpy.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rclpy/src/rclpy/_rclpy.c b/rclpy/src/rclpy/_rclpy.c index 64361d170..e9bc65179 100644 --- a/rclpy/src/rclpy/_rclpy.c +++ b/rclpy/src/rclpy/_rclpy.c @@ -160,11 +160,13 @@ rclpy_init(PyObject * Py_UNUSED(self), PyObject * args) // Expect one argument which is a list of strings PyObject * pyargs; if (!PyArg_ParseTuple(args, "O", &pyargs)) { + // Exception raised return NULL; } pyargs = PySequence_List(pyargs); if (NULL == pyargs) { + // Exception raised return NULL; } Py_ssize_t num_args = PyList_Size(pyargs); From 0dc34823b575d8dd05bddc924fcdec96ad7284c6 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Tue, 6 Mar 2018 08:40:08 -0800 Subject: [PATCH 3/4] Fix windows warning about data loss --- rclpy/src/rclpy/_rclpy.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rclpy/src/rclpy/_rclpy.c b/rclpy/src/rclpy/_rclpy.c index e9bc65179..107a42f83 100644 --- a/rclpy/src/rclpy/_rclpy.c +++ b/rclpy/src/rclpy/_rclpy.c @@ -169,7 +169,12 @@ rclpy_init(PyObject * Py_UNUSED(self), PyObject * args) // Exception raised return NULL; } - Py_ssize_t num_args = PyList_Size(pyargs); + 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); From 93c634bf20fb57def7ca3ff7517553b53d99caa3 Mon Sep 17 00:00:00 2001 From: Shane Loretz Date: Tue, 6 Mar 2018 09:05:29 -0800 Subject: [PATCH 4/4] Pass NULL when num_args == 0 --- rclpy/src/rclpy/_rclpy.c | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/rclpy/src/rclpy/_rclpy.c b/rclpy/src/rclpy/_rclpy.c index 107a42f83..77cbd9b6e 100644 --- a/rclpy/src/rclpy/_rclpy.c +++ b/rclpy/src/rclpy/_rclpy.c @@ -177,23 +177,26 @@ rclpy_init(PyObject * Py_UNUSED(self), PyObject * args) 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; - } - + char ** arg_values = 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; + if (num_args > 0) { + 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; + } + + 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); } - // Borrows a pointer, do not free arg_values[i] - arg_values[i] = PyUnicode_AsUTF8(pyarg); } if (have_args) { @@ -203,7 +206,9 @@ rclpy_init(PyObject * Py_UNUSED(self), PyObject * args) rcl_reset_error(); } } - allocator.deallocate(arg_values, allocator.state); + if (NULL != arg_values) { + allocator.deallocate(arg_values, allocator.state); + } Py_DECREF(pyargs); if (PyErr_Occurred()) {