[Python-Dev] Patch review [ 723201 ] PyArg_ParseTuple problem with 'L' format

[Michiel Jan Laurens de Hoon]

Patch review [ 723201 ] PyArg_ParseTuple problem with 'L' format

The PyArg_ParseTuple function (PyObject *args, char *format, ...) parses the
arguments args and stores them in the variables specified following the format
argument. If format=="i", indicating an integer, but the corresponding Python
object in args is not a Python int or long, a TypeError is thrown:

TypeError: an integer is required

For the "L" format, indicating a long long, instead a SystemError is thrown:

SystemError: Objects/longobject.c:788: bad argument to internal function

The submitted patch fixes this, however I think it is not the best way to do it.
The original code (part of the convertsimple function in Python/getargs.c) is

	case 'L': {/* PY_LONG_LONG */
		PY_LONG_LONG *p = va_arg( *p_va, PY_LONG_LONG * );
		PY_LONG_LONG ival = PyLong_AsLongLong( arg );
		if( ival == (PY_LONG_LONG)-1 && PyErr_Occurred() ) {
			return converterr("long<L>", arg, msgbuf, bufsize);
		} else {
			*p = ival;
		}
		break;
	}

In the patch, a PyLong_Check and a PyInt_Check are added:

         case 'L': {/* PY_LONG_LONG */
                 PY_LONG_LONG *p = va_arg(*p_va, PY_LONG_LONG *);
                 PY_LONG_LONG ival;
		/* ********** patch starts here ********** */
                 if (!PyLong_Check(arg) && !PyInt_Check(arg))
                         return converterr("long<L>", arg, msgbuf, bufsize);
		/* ********** patch ends here ********** */
                 ival = PyLong_AsLongLong(arg);
                 if (ival == (PY_LONG_LONG)-1 && PyErr_Occurred()) {
                         return converterr("long<L>", arg, msgbuf, bufsize);
                 } else {
                         *p = ival;
                 }
                 break;
         }

However, the PyLong_AsLongLong function (in Objects/longobject.c) also contains
a call to PyLong_Check and PyInt_Check, so there should be no need for another
such check here:

PY_LONG_LONG
PyLong_AsLongLong(PyObject *vv)
{
         PY_LONG_LONG bytes;
         int one = 1;
         int res;

         if (vv == NULL) {
                 PyErr_BadInternalCall();
                 return -1;
         }
         if (!PyLong_Check(vv)) {
                 if (PyInt_Check(vv))
                         return (PY_LONG_LONG)PyInt_AsLong(vv);
                 PyErr_BadInternalCall();
                 return -1;
         }

A better solution would be to replace the PyErr_BadInternalCall() in the 
PyLong_AsLongLong function by
		PyErr_SetString(PyExc_TypeError, "an integer is required");
This would make it consistent with PyInt_AsLong in Objects/intobject.c:

long
PyInt_AsLong(register PyObject *op)
{
         PyNumberMethods *nb;
         PyIntObject *io;
         long val;

         if (op && PyInt_Check(op))
                 return PyInt_AS_LONG((PyIntObject*) op);

         if (op == NULL || (nb = op->ob_type->tp_as_number) == NULL ||
             nb->nb_int == NULL) {
                 PyErr_SetString(PyExc_TypeError, "an integer is required");
                 return -1;
         }

By the way, I noticed that a Python float is converted to an int (with a
deprecation warning), while trying to convert a Python float into a long long
int results in a TypeError. Also, I'm not sure about the function of the calls 
to converterr (in various places in the convertsimple function); none of the 
argument type errors seem to lead to the warning messages created by converterr.

--Michiel.

-- 
Michiel de Hoon, Assistant Professor
University of Tokyo, Institute of Medical Science
Human Genome Center
4-6-1 Shirokane-dai, Minato-ku
Tokyo 108-8639
Japan
http://bonsai.ims.u-tokyo.ac.jp/~mdehoon