[libvirt] [PATCH] python: improve conversion validation

[Osier Yang]

On 03/31/2012 02:49 AM, Eric Blake wrote:
> Laszlo Ersek pointed out that in trying to convert a long to an
> unsigned int, we used:
>
> long long_val = ...;
> if ((unsigned int)long_val == long_val)
>
> According to C99 integer promotion rules, the if statement is
> equivalent to:
>
> (unsigned long)(unsigned int)long_val == (unsigned long)long_val
>
> since you get an unsigned comparison if at least one side is
> unsigned, using the largest rank of the two sides; but on 32-bit
> platforms, where unsigned long and unsigned int are the same size,
> this comparison is always true and ends up converting negative
> long_val into posigive unsigned int values, rather than rejecting
> the negative value as we had originally intended (python longs
> are unbounded size, and we don't want to do silent modulo
> arithmetic when converting to C code).
>
> Fix this by using direct comparisons, rather than casting.
>
> * python/typewrappers.c (libvirt_intUnwrap, libvirt_uintUnwrap)
> (libvirt_ulongUnwrap, libvirt_ulonglongUnwrap): Fix conversion
> checks.
> ---
>   python/typewrappers.c |   27 ++++++++++++++++++++-------
>   1 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/python/typewrappers.c b/python/typewrappers.c
> index af209e6..4ae3ee1 100644
> --- a/python/typewrappers.c
> +++ b/python/typewrappers.c
> @@ -132,7 +132,7 @@ libvirt_intUnwrap(PyObject *obj, int *val)
>       if ((long_val == -1)&&  PyErr_Occurred())
>           return -1;
>
> -    if ((int)long_val == long_val) {
> +    if (long_val<= INT_MAX&&  long_val>= INT_MIN) {
>           *val = long_val;
>       } else {
>           PyErr_SetString(PyExc_OverflowError,
> @@ -151,7 +151,7 @@ libvirt_uintUnwrap(PyObject *obj, unsigned int *val)
>       if ((long_val == -1)&&  PyErr_Occurred())
>           return -1;
>
> -    if ((unsigned int)long_val == long_val) {
> +    if (long_val>= 0&&  long_val<= UINT_MAX) {
>           *val = long_val;
>       } else {
>           PyErr_SetString(PyExc_OverflowError,
> @@ -183,7 +183,13 @@ libvirt_ulongUnwrap(PyObject *obj, unsigned long *val)
>       if ((long_val == -1)&&  PyErr_Occurred())
>           return -1;
>
> -    *val = long_val;
> +    if (long_val>= 0) {
> +        *val = long_val;
> +    } else {
> +        PyErr_SetString(PyExc_OverflowError,
> +                        "negative Python int cannot be converted to C unsigned long");
> +        return -1;
> +    }
>       return 0;
>   }
>
> @@ -207,16 +213,23 @@ int
>   libvirt_ulonglongUnwrap(PyObject *obj, unsigned long long *val)
>   {
>       unsigned long long ullong_val = -1;
> +    long long llong_val;
>
>       /* The PyLong_AsUnsignedLongLong doesn't check the type of
>        * obj, only accept argument of PyLong_Type, so we check it instead.
>        */
> -    if (PyInt_Check(obj))
> -        ullong_val = PyInt_AsLong(obj);
> -    else if (PyLong_Check(obj))
> +    if (PyInt_Check(obj)) {
> +        llong_val = PyInt_AsLong(obj);
> +        if (llong_val<  0)
> +            PyErr_SetString(PyExc_OverflowError,
> +                            "negative Python int cannot be converted to C unsigned long long");
> +        else
> +            ullong_val = llong_val;
> +    } else if (PyLong_Check(obj)) {
>           ullong_val = PyLong_AsUnsignedLongLong(obj);
> -    else
> +    } else {
>           PyErr_SetString(PyExc_TypeError, "an integer is required");
> +    }
>
>       if ((ullong_val == -1)&&  PyErr_Occurred())
>           return -1;

Looks good, ACK

Osier