On Monday, September 05, 2011 04:57:33 Andrej Mitrovic wrote:
> I'm looking at compare() in class TypeInfo_Array and it's defined as:
> 
> override int compare(in void* p1, in void* p2)
> {
>     void[] a1 = *cast(void[]*)p1;
>     void[] a2 = *cast(void[]*)p2;
>     size_t sz = value.tsize();
>     size_t len = a1.length;
> 
>     if (a2.length < len)
>         len = a2.length;
>     for (size_t u = 0; u < len; u++)
>     {
>         int result = value.compare(a1.ptr + u * sz, a2.ptr + u * sz);
>         if (result)
>             return result;
>     }
>     return cast(int)a1.length - cast(int)a2.length;
> }
> 
> Shouldn't that return line be:
> return cast(int)(a1.length - a2.length);
> 
> To make it 64-bit safe?l

It's more complicated than that. The result of a1.length - a2.length will always be positive, because size_t is unsigned, which won't work with compare. It should probably be something more like

if(a1.length < a2.length)
    return -1;
else
    return a.length > a2.length ? 1 : 0;

Even if you were to cast both of them to a signed version of size_t (ssize_t?), do the subtraction, and then cast the result to int, it's still wrong if you end up with values of length which can't be held in a signed version of size_t. Returning the result from a subtraction from a compare function is always buggy. It's just a question of whether you care about the possibility of overflow. And in this case, the current code is that much worse for 64-bit programs.

- Jonathan M Davis