Skip to content

MAINT: Further small return value validation fixes#20990

Merged
charris merged 1 commit intonumpy:mainfrom
seberg:fixups
Feb 3, 2022
Merged

MAINT: Further small return value validation fixes#20990
charris merged 1 commit intonumpy:mainfrom
seberg:fixups

Conversation

@seberg
Copy link
Copy Markdown
Member

@seberg seberg commented Feb 3, 2022

Half of these were probably just multiple versions of the same pattern, so it got lost.

*result = (PyObject*)PyArray_DescrNewByteorder(descr, byte_order);
Py_DECREF(descr);
if (result == NULL) {
if (*result == NULL) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Well, clang is smart enough to notice that it was always false, to bad GCC running in CI is not :)

@seberg
Copy link
Copy Markdown
Member Author

seberg commented Feb 3, 2022

Ping @charris, can you smuggle this into the release you are currently preparing?

Py_XDECREF(fields);
Py_XDECREF(nameslist);
return NULL;
goto fail;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I had not realized we had a goto here that could be used.

%#endif
if (!PyArray_IsScalar(obj,Integer)) return SWIG_TypeError;
PyArray_Descr * longDescr = PyArray_DescrNewFromType(NPY_LONG);
PyArray_Descr * longDescr = PyArray_DescrFromType(NPY_LONG);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

PyArray_DescrFromType cannot fail for builtin types (except char)... No idea if it matters, but this is fine.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes me wonder about the use of PyArray_DescrNewFromType. AFAICT, almost all uses are with builtin types.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is used mostly for voids/strings that are "mutable" so to speak (e.g. adjusting the elsize), sometimes with others to adjust byte-order.
(Or at least those are the places where it should and must be used)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are about 14 uses for types that aren't mutable. Looks like an opportunity for some cleanup.

@charris
Copy link
Copy Markdown
Member

charris commented Feb 3, 2022

can you smuggle this into the release you are currently preparing?

Yes.

@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Feb 3, 2022
@charris charris added this to the 1.22.2 release milestone Feb 3, 2022
@charris charris merged commit 9d9cc5c into numpy:main Feb 3, 2022
@charris
Copy link
Copy Markdown
Member

charris commented Feb 3, 2022

Thanks Sebastian.

@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Feb 3, 2022
@seberg seberg deleted the fixups branch February 3, 2022 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants