This is definitely an improvement. Reviewed-by: Jamey Sharp <[email protected]>
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.
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.
Jamey
On Wed, May 11, 2011 at 02:49:48PM +1000, Peter Hutterer wrote:
> diff --git a/include/misc.h b/include/misc.h
> index 803f5ba..604c893 100644
> --- a/include/misc.h
> +++ b/include/misc.h
> @@ -223,6 +223,24 @@ pad_to_int32(const int bytes) {
> extern char**
> xstrtokenize(const char *str, const char* separators);
>
> +/**
> + * Compare the two version numbers comprising of major.minor.
> + *
> + * @retval -1 if a is less than b
> + * @retval 0 if a is equal to b
> + * @retval 1 if a is greater than b
> + */
> +static inline int
> +version_compare(int a_major, int a_minor, int b_major, int b_minor)
> +{
> + int a, b;
> +
> + a = a_major * 1000 + a_minor;
> + b = b_major * 1000 + b_minor;
> +
> + return (a == b) ? 0 : (a < b) ? -1 : 1;
> +}
> +
> /* some macros to help swap requests, replies, and events */
>
> #define LengthRestB(stuff) \
signature.asc
Description: Digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
