[kamailio/kamailio] python3: `PyErr_SetString` is a void function. It doesn't return NULL. (PR #3231)
#### Pre-Submission Checklist - [x] Commit message has the format required by CONTRIBUTING guide - [x] Commits are split per component (core, individual modules, libs, utils, ...) - [ ] Each component has a single commit (if not, squash them into one commit) - [ ] No commits to README files for modules (changes must be done to docbook files in `doc/` subfolder, the README file is autogenerated)
#### Type Of Change - [ ] Small bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds new functionality) - [ ] Breaking change (fix or feature that would change existing functionality)
#### Checklist: - [ ] PR should be backported to stable branches - [ ] Tested changes locally - [ ] Related to issue #XXXX (replace XXXX with an open issue number)
#### Description
`PyErr_SetString` is a void function. It doesn't return NULL.
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/3231
-- Commit Summary --
* Python: must return NULL on error, not Py_None * modules/python3: return NULL on error.
-- File Changes --
M src/modules/app_python3/python_msgobj.c (102)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/3231.patch https://github.com/kamailio/kamailio/pull/3231.diff
Thanks for this PR!
I am not much into Python API, but somehow I get your comment in contradiction with the changes in the code, respectively:
``` PyErr_SetString is a void function. It doesn't return NULL. ```
But then the changes replace:
``` Py_INCREF(Py_None); return Py_None; ```
With a `NULL` return:
``` return NULL; ```
Is it that after use of `PyErr_SetString(...);` it is not needed to reference `Py_None`, because it was not returning a `NULL` (or the `Py_None`) object?
Since (I think), Python does not really have `NULL` inside, but the `None` keyword, what is the impact of this change returning `NULL`?
Anything particularly wrong without this patch?
Practically I am trying to figure out what exactly was wrong and this PR fixes, because the code is like this for rather long time.
Just compile the old code! You get a ton of warnings that PyErr_SetString "returns" `void`.
Python has, at heart, a very simple API. A C function **either** returns an object **or** it sets an exception (and returns NULL). Never both! (Or None for that matter.)
PyNone is a regular Python object. NULL is not, it's the API's marker for raising an exception.
Thus, the old code works perfectly well as long as it doesn't raise an exception. If/when it does, depending on the whims of the C compiler, the return value for the old version might be anything whatsoever. Even NULL if you're lucky.
Python has a ton of somewhat-higher-level error-generating functions which explicitly return NULL so that you can use the old construct but sadly PyErr_SetString is not one of these.
https://docs.python.org/3/c-api/exceptions.html
Maybe @aalba6675 wants comment as well, or someone else.
Otherwise, from my point of view, again not being a python user/devel, it is fine to merge.
Merged #3231 into master.
-
Daniel-Constantin Mierla -
Matthias Urlichs