[Python-Dev] Vestigial code in threadmodule?

[Tim Peters]

[A.M. Kuchling]
> Looking at bug #1209880, the following function from threadmodule.c is
> referenced.  I think the args==NULL case, which can return None
> instead of a Boolean value, can never be reached because
> PyArg_ParseTuple() will fail if args==NULL.

It would assert-fail in a debug build.  In a release build the most
likely outcome would be a segfault (NULL-pointer dereference in the
expansion of vgetargs1's "PyTuple_Check(args)").

> Before ripping the args==NULL code out, I wanted to be sure my
> analysis is correct; is there some subtlety here I'm missing that
> makes args==NULL possible?

Rip it out; blame me <wink>.

> --amk
>
> static PyObject *
> lock_PyThread_acquire_lock(lockobject *self, PyObject *args)

Noe that this is a file-local function.  The only references are here:

static PyMethodDef lock_methods[] = {
	{"acquire_lock", (PyCFunction)lock_PyThread_acquire_lock, 
	 METH_VARARGS, acquire_doc},
	{"acquire",      (PyCFunction)lock_PyThread_acquire_lock, 
	 METH_VARARGS, acquire_doc},

METH_VARARGS always passes a tuple (possibly empty).  These are very
old functions, so I bet they used to use METH_OLDARGS (implied by
absence at the time) ... yup, METH_VARARGS was introduced here in rev
2.48, and unintentionally changed the return contract of this
function.  So that was a backward incompatibility introduced in Python
2.3a1.  Since nobody complained then or since, I vote to keep "the
new" return contract and fiddle the docs to match it.