[Libguestfs] [PATCH 3/3] Use an unsigned type (size_t) for all loop iterators.

[Matthew Booth]

On 16/07/10 15:26, Richard W.M. Jones wrote:
> On Fri, Jul 16, 2010 at 02:27:12PM +0100, Matthew Booth wrote:
>> On 16/07/10 13:05, Richard W.M. Jones wrote:
>>> @@ -9354,7 +9354,7 @@ put_handle (guestfs_h *g)
>>>    static char **
>>>    get_string_list (PyObject *obj)
>>>    {
>>> -  int i, len;
>>> +  size_t i, len;
>>>      char **r;
>>>
>>>      assert (obj);
>>
>> This isn't safe. len is assigned later:
>>
>>    len = PyList_Size (obj);
>>
>> PyList_Size returns Py_ssize_t, which is a signed type. This may also
>> mean that the later code may also need a second look, as it doesn't
>> check for a negative return value.
>
> I checked in the Python source and PyList_Size does do some sort of
> runtime check of the obj parameter and could return -1.  No wonder
> it's slow.
>
> In the updated patch attached the hunk now looks like this:
>
> @@ -9354,7 +9354,7 @@ put_handle (guestfs_h *g)
>   static char **
>   get_string_list (PyObject *obj)
>   {
> -  int i, len;
> +  size_t i, len;
>     char **r;
>
>     assert (obj);
> @@ -9364,7 +9364,12 @@ get_string_list (PyObject *obj)
>       return NULL;
>     }
>
> -  len = PyList_Size (obj);
> +  Py_ssize_t slen = PyList_Size (obj);
> +  if (slen == -1) {
> +    PyErr_SetString (PyExc_RuntimeError, \"get_string_list: PyList_Size failure\");
> +    return NULL;
> +  }
> +  len = (size_t) slen;
>     r = malloc (sizeof (char *) * (len+1));
>     if (r == NULL) {
>       PyErr_SetString (PyExc_RuntimeError, \"get_string_list: out of memory\");
>

Looks good. ACK.

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490