> > > > On 11 May 2017, at 10:17, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > > > >> > >> For example, something like this: > >> uint8_t *p8; > >> uint32_t *p32 = (uint32_t *) p8; > >> > >> generates a warning like this: > >> spice-channel.c:1350:10: error: cast from 'uint8_t *' (aka 'unsigned char > >> *') to > >> 'uint32_t *' (aka 'unsigned int *') increases required alignment from > >> 1 > >> to > >> 4 [-Werror,-Wcast-align] > >> > >> Adding an intermediate cast to (void *) silences the warning. > >> > >> Signed-off-by: Christophe de Dinechin <dinechin@xxxxxxxxxx> > >> --- > >> spice-common | 2 +- > >> src/channel-cursor.c | 6 +++--- > >> src/channel-display-mjpeg.c | 2 +- > >> src/decode-glz-tmpl.c | 2 +- > >> src/spice-channel.c | 14 +++++++++----- > >> 5 files changed, 15 insertions(+), 11 deletions(-) > >> > >> diff --git a/spice-common b/spice-common > >> index af682b1..1239c82 160000 > >> --- a/spice-common > >> +++ b/spice-common > >> @@ -1 +1 @@ > >> -Subproject commit af682b1b06dea55007d9aa7c37cd443e4349e43f > >> +Subproject commit 1239c82c54dee0244cca262cca0aa21071b23e24 > >> diff --git a/src/channel-cursor.c b/src/channel-cursor.c > >> index 609243b..51c6c2e 100644 > >> --- a/src/channel-cursor.c > >> +++ b/src/channel-cursor.c > >> @@ -340,7 +340,7 @@ static display_cursor *set_cursor(SpiceChannel > >> *channel, > >> SpiceCursor *scursor) > >> memcpy(cursor->data, data, size); > >> for (i = 0; i < hdr->width * hdr->height; i++) { > >> pix_mask = get_pix_mask(data, size, i); > >> - if (pix_mask && *((guint32*)data + i) == 0xffffff) { > >> + if (pix_mask && *(SPICE_ALIGNED_CAST(guint32*, data) + i) == > >> 0xffffff) { > >> cursor->data[i] = get_pix_hack(i, hdr->width); > >> } else { > >> cursor->data[i] |= (pix_mask ? 0 : 0xff000000); > >> @@ -350,7 +350,7 @@ static display_cursor *set_cursor(SpiceChannel > >> *channel, > >> SpiceCursor *scursor) > >> case SPICE_CURSOR_TYPE_COLOR16: > >> for (i = 0; i < hdr->width * hdr->height; i++) { > >> pix_mask = get_pix_mask(data, size, i); > >> - pix = *((guint16*)data + i); > >> + pix = *(SPICE_ALIGNED_CAST(guint16*,data) + i); > >> if (pix_mask && pix == 0x7fff) { > >> cursor->data[i] = get_pix_hack(i, hdr->width); > >> } else { > >> @@ -364,7 +364,7 @@ static display_cursor *set_cursor(SpiceChannel > >> *channel, > >> SpiceCursor *scursor) > >> for (i = 0; i < hdr->width * hdr->height; i++) { > >> pix_mask = get_pix_mask(data, size + (sizeof(uint32_t) << 4), > >> i); > >> int idx = (i & 1) ? (data[i >> 1] & 0x0f) : ((data[i >> 1] & > >> 0xf0) >> 4); > >> - pix = *((uint32_t*)(data + size) + idx); > >> + pix = *(SPICE_ALIGNED_CAST(uint32_t*,(data + size)) + idx); > >> if (pix_mask && pix == 0xffffff) { > >> cursor->data[i] = get_pix_hack(i, hdr->width); > >> } else { > > > > This is not fine. The palette at the end of the image (this is a 4 bit > > image) > > is not garantee to be 4 byte aligned. > > OK. This would show up as a warning in the logs, but I guess I would need to > test color cursor changes to see it, right? > actually you need a 4 bit cursor, not sure if windows use these anymore... surely with an old windows version you'll get these. I supposed is harder to find on X. > > > > >> diff --git a/src/channel-display-mjpeg.c b/src/channel-display-mjpeg.c > >> index 3ae9d21..17c0f4f 100644 > >> --- a/src/channel-display-mjpeg.c > >> +++ b/src/channel-display-mjpeg.c > >> @@ -151,7 +151,7 @@ static gboolean mjpeg_decoder_decode_frame(gpointer > >> video_decoder) > >> #ifndef JCS_EXTENSIONS > >> { > >> uint8_t *s = lines[0]; > >> - uint32_t *d = (uint32_t *)s; > >> + uint32_t *d = SPICE_ALIGNED_CAST(uint32_t *,s); > >> > >> if (back_compat) { > >> for (unsigned int j = lines_read * width; j > 0; ) { > > > > Not really sure, lines are returned by jpeg_read_scanlines, not sure > > the output alignment. > > I prefer to keep “aligned” by default. If there is a warning showing up in > the logs, we can switch it to unaligned. > Well.. no as actually are just macros silencing a possible problem would be better if this could be unaligned so a grep would help the real porting (if needed) in the future. > > > >> diff --git a/src/decode-glz-tmpl.c b/src/decode-glz-tmpl.c > >> index b337a8b..7695a28 100644 > >> --- a/src/decode-glz-tmpl.c > >> +++ b/src/decode-glz-tmpl.c > >> @@ -178,7 +178,7 @@ static size_t FNAME(decode)(SpiceGlzDecoderWindow > >> *window, > >> uint64_t image_id, SpicePalette *plt) > >> { > >> uint8_t *ip = in_buf; > >> - OUT_PIXEL *out_pix_buf = (OUT_PIXEL *)out_buf; > >> + OUT_PIXEL *out_pix_buf = SPICE_ALIGNED_CAST(OUT_PIXEL *,out_buf); > >> OUT_PIXEL *op = out_pix_buf; > >> OUT_PIXEL *op_limit = out_pix_buf + size; > >> > >> diff --git a/src/spice-channel.c b/src/spice-channel.c > >> index 3b6231e..88d0567 100644 > >> --- a/src/spice-channel.c > >> +++ b/src/spice-channel.c > >> @@ -1312,6 +1312,7 @@ static void spice_channel_send_link(SpiceChannel > >> *channel) > >> { > >> SpiceChannelPrivate *c = channel->priv; > >> uint8_t *buffer, *p; > >> + uint32_t *p32; > >> int protocol, i; > >> > >> c->link_hdr.magic = SPICE_MAGIC; > >> @@ -1356,14 +1357,15 @@ static void spice_channel_send_link(SpiceChannel > >> *channel) > >> memcpy(p, &c->link_hdr, sizeof(c->link_hdr)); p += sizeof(c->link_hdr); > >> memcpy(p, &c->link_msg, sizeof(c->link_msg)); p += sizeof(c->link_msg); > >> > >> + // Need this to avoid error with -Wcast-align > >> + p32 = SPICE_UNALIGNED_CAST(uint32_t *,p); > > > > > > absolutely false! This is SURELY not aligned, link_msg is aligned to 2 > > bytes. > > This is why it is marked as “unaligned”. This one did indeed show up in my > experiments. > Though were all "aligned"... my mistake. Yes, there are right. Just had coffee, should help :-) > > > >> for (i = 0; i < c->common_caps->len; i++) { > >> - *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->common_caps, > >> uint32_t, i)); > >> - p += sizeof(uint32_t); > >> + *p32++ = GUINT32_TO_LE(g_array_index(c->common_caps, uint32_t, > >> i)); > >> } > >> for (i = 0; i < c->caps->len; i++) { > >> - *(uint32_t *)p = GUINT32_TO_LE(g_array_index(c->caps, uint32_t, > >> i)); > >> - p += sizeof(uint32_t); > >> + *p32++ = GUINT32_TO_LE(g_array_index(c->caps, uint32_t, i)); > >> } > >> + p = (uint8_t *) p32; > >> CHANNEL_DEBUG(channel, "channel type %d id %d num common caps %u num > >> caps > >> %u", > >> c->channel_type, > >> c->channel_id, > >> @@ -1887,6 +1889,7 @@ static gboolean > >> spice_channel_recv_link_msg(SpiceChannel *channel) > >> SpiceChannelPrivate *c; > >> int rc, num_caps, i; > >> uint32_t *caps, num_channel_caps, num_common_caps; > >> + uint8_t *caps_src; > >> SpiceChannelEvent event = SPICE_CHANNEL_ERROR_LINK; > >> > >> g_return_val_if_fail(channel != NULL, FALSE); > >> @@ -1926,7 +1929,8 @@ static gboolean > >> spice_channel_recv_link_msg(SpiceChannel *channel) > >> /* see original spice/client code: */ > >> /* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * > >> sizeof(uint32_t) > c->peer_msg + c->peer_hdr.size); */ > >> > >> - caps = (uint32_t *)((uint8_t *)c->peer_msg + > >> GUINT32_FROM_LE(c->peer_msg->caps_offset)); > >> + caps_src = (uint8_t *)c->peer_msg + > >> GUINT32_FROM_LE(c->peer_msg->caps_offset); > >> + caps = SPICE_UNALIGNED_CAST(uint32_t *, caps_src); > >> > > > > same! > > Are you saying this one is guaranteed to be unaligned? If so, that’s the > correct cast. > > > > >> g_array_set_size(c->remote_common_caps, num_common_caps); > >> for (i = 0; i < num_common_caps; i++, caps++) { > > I cannot find these macros defined in spice-gtk. Where are they? Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel