[PATCH wayland] make wl_fixed_t a safe type

[Pekka Paalanen]

On Mon,  4 Jun 2012 17:48:16 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> Turn wl_fixed_t into a struct type, so that it will no longer implicitly
> cast into any other type.
> 
> Silent implicit casts between wl_fixed_t and all integer types have
> hidden many bugs, that the compiler could have easily cought.
> 
> Signed-off-by: Pekka Paalanen <ppaalanen at gmail.com>
> ---
>  src/connection.c        |    7 ++++---
>  src/wayland-util.h      |   39 +++++++++++++++++++++++++++++++--------
>  tests/connection-test.c |   19 ++++++++++---------
>  tests/fixed-benchmark.c |   14 +++++++-------
>  tests/fixed-test.c      |   42 +++++++++++++++++++++---------------------
>  5 files changed, 73 insertions(+), 48 deletions(-)
> 
...
> diff --git a/src/wayland-util.h b/src/wayland-util.h
> index 8447640..b3495fe 100644
> --- a/src/wayland-util.h
> +++ b/src/wayland-util.h
> @@ -167,17 +167,19 @@ void wl_array_release(struct wl_array *array);
>  void *wl_array_add(struct wl_array *array, size_t size);
>  void wl_array_copy(struct wl_array *array, struct wl_array *source);
>  
> -typedef int32_t wl_fixed_t;
> +typedef struct {
> +	int32_t raw;
> +} wl_fixed_t;
>  
>  static inline double
> -wl_fixed_to_double (wl_fixed_t f)
> +wl_fixed_to_double(wl_fixed_t f)
>  {
>  	union {
>  		double d;
>  		int64_t i;
>  	} u;
>  
> -	u.i = ((1023LL + 44LL) << 52) + (1LL << 51) + f;
> +	u.i = ((1023LL + 44LL) << 52) + (1LL << 51) + f.raw;
>  
>  	return u.d - (3LL << 43);
>  }
> @@ -185,23 +187,44 @@ wl_fixed_to_double (wl_fixed_t f)
>  static inline wl_fixed_t
>  wl_fixed_from_double(double d)
>  {
> +	wl_fixed_t f;
>  	union {
>  		double d;
>  		int64_t i;
>  	} u;
>  
>  	u.d = d + (3LL << (51 - 8));
> +	f.raw = u.i;
>  
> -	return u.i;
> +	return f;
>  }
>  
> -static inline int wl_fixed_to_int(wl_fixed_t f)
> +static inline int
> +wl_fixed_to_int(wl_fixed_t f)
>  {
> -	return f / 256;
> +	return f.raw / 256;
>  }
> -static inline wl_fixed_t wl_fixed_from_int(int i)
> +
> +static inline wl_fixed_t
> +wl_fixed_from_int(int i)
> +{
> +	wl_fixed_t f;
> +	f.raw = i * 256;
> +	return f;
> +}
> +
> +static inline wl_fixed_t
> +wl_fixed_add(wl_fixed_t a, wl_fixed_t b)
> +{
> +	wl_fixed_t result = { a.raw + b.raw };
> +	return result;
> +}
> +
> +static inline wl_fixed_t
> +wl_fixed_sub(wl_fixed_t a, wl_fixed_t b)
>  {
> -	return i * 256;
> +	wl_fixed_t result = { a.raw - b.raw };
> +	return result;
>  }
>  
>  typedef void (*wl_log_func_t)(const char *, va_list);

Hi Kristian,

should I rebase and re-submit this series?
Or is this NAK'd?

It affects libwayland API, so it cannot be done after 1.0.

Is there perhaps some GCC magic we could use to get warnings about
implicit casts only with wl_fixed_t?

Could 'sparse' be used for this?


Thanks,
pq