GH-132775: Fix argument parsing for _interpqueues.put()
#137686
Conversation
Modules/_interpqueuesmodule.c
Outdated
| @@ -1603,7 +1603,7 @@ queuesmod_put(PyObject *self, PyObject *args, PyObject *kwds) | |||
| PyObject *obj; | |||
| int unboundarg = -1; | |||
| int fallbackarg = -1; | |||
| if (!PyArg_ParseTupleAndKeywords(args, kwds, "O&O|ii$p:put", kwlist, | |||
| if (!PyArg_ParseTupleAndKeywords(args, kwds, "O&O|ii:put", kwlist, | |||
There was a problem hiding this comment.
I think that this is the correct solution:
| if (!PyArg_ParseTupleAndKeywords(args, kwds, "O&O|ii:put", kwlist, | |
| if (!PyArg_ParseTupleAndKeywords(args, kwds, "O&O|i$p:put", kwlist, |
Please add tests with the fallback argument equal to None, -1, 2**1000 -- there should be different behavior for patched and unpatched code. Also, fallback is now keyword-only. And passing unsupported keyword argument could crash the old code.
There was a problem hiding this comment.
I went with 'i' because that's what .create() uses, but I'm now not sure which is more correct. @ericsnowcurrently?
There was a problem hiding this comment.
#134440 removed fmt ("i") and added keyword-only boolean fallback ("p"). We usually make booleans keyword-only.
fallback has only three state -- _PyXIDATA_XIDATA_ONLY (0), _PyXIDATA_FULL_FALLBACK (1), and default negative value used to distinguish "no argument" from passed value. Other positive values are errors.
If it was also added in other functions, it most likely should be keyword-only boolean too.
There was a problem hiding this comment.
I've done as you suggest. Note the changes I had to make to .create(), though.
It would be good to get this in to 3.14rc2 (now this Thu). Feel free to take over the branch.
There was a problem hiding this comment.
Unfortunately, queues.put() is never called with the fallback argument, so we do not have examples of non-default values. queues.create() is only called with the default value -1. Maybe @ericsnowcurrently planned to add named constants for fallback, as for unbound. Or he started with integers and planned to make it boolean. Only he can answer these questions, we do not have enough information.
Bug introduced in #134440, cc @serhiy-storchaka @ericsnowcurrently. Also xref gh-137685.
A