On the system I'm porting to(*), an application will trap if the caller does not pass the exact parameter list that the callee requires. This is causing problems running Python.

One common instance where this appears to be causing problems is where functions are registered as METH_NOARGS methods. For example, in Obejcts/dictobject.c, dict_popitem() is declared:

static PyObject *dict_popitem(dictobject *mp);

However, as it is declared in the method array as METH_NOARGS, it will be called by Objects/methodobject.c:PyCFunction_Call() as "(*meth)(self, NULL)" (i.e., an extra NULL parameter is passed for some reason). This will fail on my target system.

I've no problem submitting a patch for this (dictobject.c is by no means the only place this is happening - it's just the first one encountered because it's used so much - though some places _do_ correctly declare a second, ignored parameter). However, I'd like to get agreement on the correct form it should be changed to before I put the effort in to produce a patch (it's going to be a fairly tedious process to identify and fix all these).

In various modules, the functions are called internally as well as being registered as METH_NOARGS methods.

Therefore, the change can either be:

static PyObject *foo(PyObject *self)
{
...
}

static PyObject *foo_noargs(PyObject *self, void *noargs_null)
{
   return foo(self);
}

... where 'foo' is called internally and 'foo_noargs' is registered as a METH_NOARGS method.

or:

static PyObject *foo(PyObject *self, void *noargs_null)
{
...
}

... and any internal calls in the module have to pass a second, NULL, argument in each call.

The former favours internal module calls over METH_NOARGS calls, the latter penalises them. Which is preferred? Should this be raised on a different forum? Does anyone care? ;)

Thanks, Kev.

(*) Details on request.

The current specification says that these should be PyCFunction pointers, see

http://docs.python.org/api/common-structs.html

My initial implementation of METH_NOARGS had it differently, and nobody ever bothered fixing them all when this was changed. Please do submit a patch to correct all such errors, both in code and documentation.

File Added: tested.diff

Hi,

I am submitting two patches (both against the 2.5 release sources). One
contains a set of changes which have subsequently been compiled by me and
used to run lib/python/test/regrtest.py. If the format of the changes
themselves is acceptable, then I believe this patch can be applied relatively
confidently. I haven't paid too much attention to conditional compilation in
those files, but there appears to be little in the areas I've touched.

The second contains a set of changes to source files that are not being used
at present on my system. Therefore, they _may_ not compile. I have visually
checked that all functions whose signature I have changed are not called
directly (across all source files) with the old signature and have also checked
header file prototypes. However, that doesn't mean I didn't miss something, so
this patch should be applied with a little more care.

The nature of the fixes themselves are discussed below.

-----------------------------------
==== Fixes to common problems across several files:

• Failure to declare second (always NULL) parameter on functions registered as
  METH_NOARGS methods.

  • These all now have a second parameter declared as "PyObject *NOARGS_NULL".
  • I have also changed ones that already declared the parameter as
    "void *ignored" etc, as I think the name makes it clear why it's there.
    If the upper-case name is bad style, feel free to change it to something
    else - as they are all now consistent, that should be a trivial process
    to change in the patch file before applying it.

• PyGetSetDef 'getter' and 'setter' functions not declaring the final 'closure'
  parameter.

  • These all now have a final parameter declared as "void *closure".
  • I have also changed ones that already declared the parameter as
    "void *context" or "void *ignored" etc, for consistency.

• The tp_clear type slot is defined as type 'inquiry' but the return value is
  ignored and in some instances, not returned at all. This is related to the
  following thread:

  http://mail.python.org/pipermail/python-dev/2003-April/034433.html

  frameobject.c and traceback.c were either missed when those changes were
  made, or the problems were re-introduced since.

  • I have changed the functions in those files to return zero.

==== Miscellaneous individual fixes:

• Objects/fileobject.c:file_self() is registered both in the "tp_iter" slot
  and as a METH_NOARGS function. The "tp_iter" slot function is
  called with one parameter (the object) and the METH_NOARGS function is called
  with two parameters (the object, a NULL pointer).

  • Wrapper function file_self_noargs() created which accepts the additional
    "PyObject *NOARGS_NULL" parameter and just calls the file_self() function.
  • All other occurences of tp_iter visually checked and appear to be OK.

• The datetimemodule.c problem with time_isoformat() being registered as
  METH_KEYWORDS instead of METH_NOARGS is also fixed here, though I believe
  that has already been dealt with.

-----------------------------------

All in all, that was a pretty tedious process! Hopefully these changes can
mostly make it in so I don't have to do it all over again one day ;)

Regards, Kev.

File Added: untested.diff

Kev, I would be interested to know the platform this was a problem on.

I haven't looked at the patch much (a little of tested.diff), primarily Martin's msg on python-dev. I'm in favor of fixing this in concept. I tend to agree with Thomas that the parameter name in ALL_CAPS seems a bit annoying. I don't have a better suggestion and would rather see the patch applied with ALL_CAPS than not applied.

I would also like to remove the casts to PyCFunction for all the functions that are stored in the various static structures. That will help ensure we don't copy bad examples and propagate the problem in the future.

It appears that in principle this patch is acceptable but I believe that it will need pretty thorough checking from a core developer before going anywhere. It's certainly not in my league, anyone up for it?

This issue has been partially (for METH_NOARGS methods) fixed in bpo-33012.

This appears to be fixed for dict_popitem, as Serhyi mentioned above. I believe the WASM porting effort fixed a few other instances, because WASM is strict about this. I will close this issue; if any concrete places come up where this is still a problem, please open a more focused issue.