[Spice-devel] [spice-common PATCH v1 8/12] marshaller: return void* to reserve_space function

[Frediano Ziglio]

> 
> besides clang yelling about alignment below, we do guarantee reserved
> space of size_t so returning void* should be okay.
> 
> char_device.c:923:29: error: cast from 'uint8_t *' (aka 'unsigned char *')
> to 'uint32_t *' (aka 'unsigned int *') increases required alignment
> from 1 to 4 [-Werror,-Wcast-align]
> 
> write_to_dev_size_ptr = (uint32_t *)spice_marshaller_reserve_space(m,
> sizeof(uint32_t));
>                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ---
>  common/marshaller.c | 2 +-
>  common/marshaller.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/marshaller.c b/common/marshaller.c
> index bd012d7..53c1c1c 100644
> --- a/common/marshaller.c
> +++ b/common/marshaller.c
> @@ -238,7 +238,7 @@ static size_t remaining_buffer_size(SpiceMarshallerData
> *d)
>      return MARSHALLER_BUFFER_SIZE - d->current_buffer_position;
>  }
>  
> -uint8_t *spice_marshaller_reserve_space(SpiceMarshaller *m, size_t size)
> +void *spice_marshaller_reserve_space(SpiceMarshaller *m, size_t size)
>  {
>      MarshallerItem *item;
>      SpiceMarshallerData *d;
> diff --git a/common/marshaller.h b/common/marshaller.h
> index e19c0f6..1f43bba 100644
> --- a/common/marshaller.h
> +++ b/common/marshaller.h
> @@ -34,7 +34,7 @@ typedef void (*spice_marshaller_item_free_func)(uint8_t
> *data, void *opaque);
>  SpiceMarshaller *spice_marshaller_new(void);
>  void spice_marshaller_reset(SpiceMarshaller *m);
>  void spice_marshaller_destroy(SpiceMarshaller *m);
> -uint8_t *spice_marshaller_reserve_space(SpiceMarshaller *m, size_t size);
> +void *spice_marshaller_reserve_space(SpiceMarshaller *m, size_t size);
>  void spice_marshaller_unreserve_space(SpiceMarshaller *m, size_t size);
>  uint8_t *spice_marshaller_add(SpiceMarshaller *m, const uint8_t *data,
>  size_t size);
>  uint8_t *spice_marshaller_add_ref(SpiceMarshaller *m, uint8_t *data, size_t
>  size);
> --
> 2.4.3
> 

No, spice_marshaller_reserve_space can return unaligned data so returning uint8_t* is fine.
The compiler is correctly complaining about a possible problem. Some architecture have problems
(core!) if data are not correctly aligned or some compiler declaration is used.
In this case the code is not safe as it is.

Probably code calling macro like this

#define write_int16(ptr,v) (*((int16_t *)(ptr)) = v)

generate the same (correct) warning.

You should define some unaligned type like

typedef uint32_t __attribute__ ((aligned(1))) uint32u_t;

Frediano