Skip to content

Commit 7077033

Browse files
authored
Revamp how we check for the correct class. (#218)
* Revamp how we check for the correct class. In an issue it was pointed out that we are decref'ing before we actually use some of the data, which means that in theory the garbage collector could reclaim the data before we used it. So the original point of this change was to fix that issue. However, while looking at it I realized we could slightly improve performance here by avoiding a copy of the class and module into a combined string. Instead, we can compare them separately, which should reduce the copies. * Fix a warning when building in Release mode. Since the variables are only used in an assert, which can be compiled out, this could lead to a warning. Inline the call instead, which should get rid of the warning. Signed-off-by: Chris Lalancette <[email protected]>
1 parent acf97aa commit 7077033

File tree

1 file changed

+24
-28
lines changed

1 file changed

+24
-28
lines changed

rosidl_generator_py/resource/_msg_support.c.em

Lines changed: 24 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -158,41 +158,37 @@ PyObject * @('__'.join(type_.namespaces + [convert_camel_case_to_lower_case_unde
158158

159159
@{
160160
module_name = '_' + convert_camel_case_to_lower_case_underscore(interface_path.stem)
161+
class_module = '%s.%s' % ('.'.join(message.structure.namespaced_type.namespaces), module_name)
162+
namespaced_type = message.structure.namespaced_type.name
161163
}@
162164
ROSIDL_GENERATOR_C_EXPORT
163165
bool @('__'.join(message.structure.namespaced_type.namespaces + [convert_camel_case_to_lower_case_underscore(message.structure.namespaced_type.name)]))__convert_from_py(PyObject * _pymsg, void * _ros_message)
164166
{
165-
@{
166-
full_classname = '%s.%s.%s' % ('.'.join(message.structure.namespaced_type.namespaces), module_name, message.structure.namespaced_type.name)
167-
}@
168167
// check that the passed message is of the expected Python class
169168
{
170-
char full_classname_dest[@(len(full_classname) + 1)];
171-
{
172-
char * class_name = NULL;
173-
char * module_name = NULL;
174-
{
175-
PyObject * class_attr = PyObject_GetAttrString(_pymsg, "__class__");
176-
if (class_attr) {
177-
PyObject * name_attr = PyObject_GetAttrString(class_attr, "__name__");
178-
if (name_attr) {
179-
class_name = (char *)PyUnicode_1BYTE_DATA(name_attr);
180-
Py_DECREF(name_attr);
181-
}
182-
PyObject * module_attr = PyObject_GetAttrString(class_attr, "__module__");
183-
if (module_attr) {
184-
module_name = (char *)PyUnicode_1BYTE_DATA(module_attr);
185-
Py_DECREF(module_attr);
186-
}
187-
Py_DECREF(class_attr);
188-
}
189-
}
190-
if (!class_name || !module_name) {
191-
return false;
192-
}
193-
snprintf(full_classname_dest, sizeof(full_classname_dest), "%s.%s", module_name, class_name);
169+
PyObject * class_attr = PyObject_GetAttrString(_pymsg, "__class__");
170+
if (class_attr == NULL) {
171+
return false;
172+
}
173+
PyObject * name_attr = PyObject_GetAttrString(class_attr, "__name__");
174+
if (name_attr == NULL) {
175+
Py_DECREF(class_attr);
176+
return false;
194177
}
195-
assert(strncmp("@(full_classname)", full_classname_dest, @(len(full_classname))) == 0);
178+
PyObject * module_attr = PyObject_GetAttrString(class_attr, "__module__");
179+
if (module_attr == NULL) {
180+
Py_DECREF(name_attr);
181+
Py_DECREF(class_attr);
182+
return false;
183+
}
184+
185+
// PyUnicode_1BYTE_DATA is just a cast
186+
assert(strncmp("@(class_module)", (char *)PyUnicode_1BYTE_DATA(module_attr), @(len(class_module))) == 0);
187+
assert(strncmp("@(namespaced_type)", (char *)PyUnicode_1BYTE_DATA(name_attr), @(len(namespaced_type))) == 0);
188+
189+
Py_DECREF(module_attr);
190+
Py_DECREF(name_attr);
191+
Py_DECREF(class_attr);
196192
}
197193
@(msg_typename) * ros_message = _ros_message;
198194
@[for member in message.structure.members]@

0 commit comments

Comments
 (0)