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

Jamey Sharp jamey at minilop.net
Wed May 11 14:40:39 PDT 2011


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.

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) \
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110511/35a85574/attachment.pgp>


More information about the xorg-devel mailing list