color_match in dlls/d3d8/tests/visual.c may not do what one would think

Gerald Pfeifer gerald at pfeifer.com
Tue Nov 6 18:10:49 CST 2018


On Tue, 6 Nov 2018, Henri Verbeet wrote:
>>    static BOOL color_match(D3DCOLOR c1, D3DCOLOR c2, BYTE max_diff)
>>    {
>>        if (abs((c1 & 0xff) - (c2 & 0xff)) > max_diff) return FALSE;
>> Substraction of two unsigned integer types is done in unsigned
>> math, so we may be seeing wrapping, but never a negative number
>> as a result -- and abs() is a noop.
> As Ken pointed out, the code has been in the tree a little longer than
> this year. As for the issue you point out, it's true that the function
> assumes sizeof(DWORD) == sizeof(int), and a two's complement system.
> I.e., in two's complement 0u - 1u == ~0u == -1, etc. I don't think
> those are unreasonable assumptions. Perhaps the function has room for
> improvement, but I'd prefer to not simply sprinkle some casts on top.

Thanks for looking into this and the explanation.  Since we are in 
unsigned land, the abs() really is a noop, so at a minimum should 
be removed?  I'll submit a patch in a minute.

Gerald



More information about the wine-devel mailing list