Hi, On Thu, 2 Jul 2009 09:48:48 +1000, René Dudfield <[email protected]> wrote: > Cool, thanks for the testing. > > > On Thu, Jul 2, 2009 at 9:19 AM, Lorenz Quack<[email protected]> wrote: >> Ok, so here goes my analysis of the failures in ColorTypeTest: >> In src/color.c line 1435 (and some following lines) we bit shift each > color >> component and stuff them >> in an unsigned long. > > Maybe this should be into a uint32 instead? >
Yes, that would correspond to my solution 1) only better because it ensures that it will have exactly 32 bit. BTW, are these types uint32, UInt8, ... in the C-standard? yours //Lorenz > >> I think the problem arises when we bitshift the r value and r is >127 >> My guess is that C converts r to an integer during the bitshift and not > to >> an unsigned int so >> if r > 127 the integer (r is shifted by 24) overflows and becomes > negative >> (note that the internal >> bit representation of the int has not changed) but when we store this >> (negative) integer into a >> long it remains negative so the first bits get filled up with "1"s. >> I see different solutions to this and cant decide which is the _correct_ > one >> so I'll just present them >> instead: >> 1) don't save the combined color in an unsigned long. use a unsigned > int >> instead. >> This should work as long as r, g, b, a are UInt8 and aren't shifted > more >> than 24 >> 2) explicitly cast the r value to be unsigned >> 3) cast the r value to be (unsigned) long >> >> furthermore I think in the we should compare tmp to UINT_MAX in line > 1438, >> 1476 and 1492. >> even though I haven't put much thought into this at least then the tests >> don't fail. >>
