[Patches] [ python-Patches-1648268 ] Parameter list mismatches (portation problem)

Sat Feb 24 17:19:34 CET 2007

Patches item #1648268, was opened at 2007-01-30 14:15
Message generated for change (Comment added) made by nnorwitz
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1648268&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: Python 2.5
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: ked-tao (ked-tao)
Assigned to: Nobody/Anonymous (nobody)
Summary: Parameter list mismatches (portation problem)

Initial Comment:
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.

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

>Comment By: Neal Norwitz (nnorwitz)
Date: 2007-02-24 08:19

Message:
Logged In: YES 
user_id=33168
Originator: NO

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.

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

Comment By: ked-tao (ked-tao)
Date: 2007-02-16 08:46

Message:
Logged In: YES 
user_id=1703158
Originator: YES

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

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

Comment By: ked-tao (ked-tao)
Date: 2007-02-16 08:42

Message:
Logged In: YES 
user_id=1703158
Originator: YES

File Added: tested.diff

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

Comment By: Martin v. Löwis (loewis)
Date: 2007-02-06 11:49

Message:
Logged In: YES 
user_id=21627
Originator: NO

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.

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

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1648268&group_id=5470