[PATCH 09/11] include: add version_compare helper function

Peter Hutterer peter.hutterer at who-t.net
Wed May 11 18:08:24 PDT 2011


On Wed, May 11, 2011 at 02:40:39PM -0700, Jamey Sharp wrote:
> This is definitely an improvement.
> 
> Reviewed-by: Jamey Sharp <jamey at minilop.net>
> 
> I'd suggest weakening the post-conditions of version_compare though, to
> match the semantics of functions like strcmp: If a is less than b,
> promise only that the return value is something less than 0, and
> similarly for greater. That permits an implementation that simply
> returns (a - b), provided that a and b both fit in 31 bits.

amended, see patch below.
I added your Reviewed-by tag too, even though there were a bit of changes,
please re-confirm though.

> I'd also change the parameter types to CARD8. The implementation is
> wrong if a_minor or b_minor are ever outside the range [0,999], which is
> OK in practice because these values come from CARD8s. Just make that
> assumption explicit in the types.

> Finally, given that the minor versions are in fact CARD8s, I'd replace
> the multiply/add with shift/or. When I first glanced at "a_major*1000",
> it wasn't obvious where that number came from. Shifting by eight bits
> instead clearly goes along with the CARD8 argument type.

XIQueryVersion requires uint16_t, so I just changed it to that, with a <<
16. and a test case, so we can make sure that 0 is always smaller than 1 and
what not.

Cheers,
  Peter
 



More information about the xorg-devel mailing list