Copilot commented on code in PR #50409:
URL: https://github.com/apache/arrow/pull/50409#discussion_r3588883888
##########
python/pyarrow/src/arrow/python/python_to_arrow.cc:
##########
@@ -1050,8 +1049,8 @@ class PyStructConverter : public
StructConverter<PyConverter, PyConverterTrait>
PyObject* unicode =
PyUnicode_FromStringAndSize(field_name.c_str(), field_name.size());
RETURN_IF_PYERROR();
- PyList_SET_ITEM(bytes_field_names_.obj(), i, bytes);
- PyList_SET_ITEM(unicode_field_names_.obj(), i, unicode);
+ PyList_SetItem(bytes_field_names_.obj(), i, bytes);
+ PyList_SetItem(unicode_field_names_.obj(), i, unicode);
Review Comment:
`PyList_SetItem` returns a status code and can set a Python exception on
failure. Since the return value is ignored and there's no `RETURN_IF_PYERROR()`
after these calls, an unexpected failure would be silently dropped (and the
newly created objects may not be DECREF’d if the list didn’t steal them).
Please check the return values and propagate errors consistently (similar to
patterns elsewhere in this codebase).
##########
python/pyarrow/src/arrow/python/python_to_arrow.cc:
##########
@@ -1266,7 +1265,7 @@ Status ConvertToSequenceAndInferSize(PyObject* obj,
PyObject** seq, int64_t* siz
RETURN_IF_PYERROR();
break;
}
- PyList_SET_ITEM(lst, i, item);
+ PyList_SetItem(lst, i, item);
Review Comment:
`PyList_SetItem`’s return value is ignored. If it ever fails, the exception
would not be propagated and local refs may leak (since the list only steals
`item` on success). Please handle the return value and bail out on error
(DECREF’ing local refs as needed).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
