On Friday 25 July 2008 01:12:55 Christoph Otto via RT wrote:
> According to make cover, the case where the return was -1 was never hit.
> A simple test case caused a segfault, which definitely isn't the right
> thing. The attached patch fixes and refactors intlist_get() to make the
> coverage more obvious in the output of the coverage tests. It also adds
> a test case to exercise the previously neglected path.
>
> I'd like comments on whether intlist_get does the right thing with this
> patch and if it's exercised correctly by the test.
IntList PMCs can resize, so it looks correct to me.
> Index: src/intlist.c
> ===================================================================
> --- src/intlist.c (revision 29733)
> +++ src/intlist.c (working copy)
> @@ -306,10 +306,13 @@
> INTVAL
> intlist_get(PARROT_INTERP, ARGMOD(IntList *list), INTVAL idx)
> {
> - /* XXX list_get can return NULL RT #48367 */
> void * const ret = list_get(interp, (List *)list, idx,
> enum_type_INTVAL); - const INTVAL retval = ret == (void *)-1 ? 0 :
> *(INTVAL *)ret; - return retval;
> + if (ret == NULL) {
> + return (INTVAL)0;
> + }
> + else {
> + return *(INTVAL *)ret;
> + }
> }
I'd write that as:
if (ret)
return *(INTVAL *)ret;
return (INTVAL)0;
The pointer casting dereferencing bothers me a little, but if compilers don't
warn about it....
-- c
