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

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\");