Re: [PATCH spice-protocol 1/2 v2] qxl_dev: Fix alignment for QXLReleaseInfo

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/18/19 5:51 PM, Frediano Ziglio wrote:
Hi,


IIRC this was related to some compiler warning, no?
Yes, recent compilers are reporting it, see below.

If it is I'd mentioning it , otherwise, ack.

Just this patch or the entire series?


No, actually i started looking at the second one and wondered why
we are using #include end/start-packed.h

It is mentioned in start-packed.h it's because "its not possible to put pragmas into header files"
and just after that we put pragma, into this header file.
What am i missing? or the comment is wrong?



Snir.


On 7/4/19 4:56 PM, Frediano Ziglio wrote:
Do not declare the structure as aligned.
The start/end-packed.h headers affects structures without
specification only using MingW or Microsoft compilers. For other
platform SPICE_ATTR_PACKED macro should be used.  This way the
definition are the same for all compiler.
This structure is used in a lot of QXL structures which are not
aligned causing to have an aligned structure to be potentially
unaligned.
What about changing this paragraph to:

"This structure is used in a lot of QXL structures which are not
  aligned causing to have an aligned structure to be potentially
  unaligned. Some compilers report a warning for some usage."


Not a big deal but maybe "Some compilers may report a warning"?

Anyway, ack for the change.


As this structure has no holes this change does not make any size
change using any compiler.
The change will only change the alignment from 4/8 to 1.
This could affect structures containing this union however beside
packed structure in qxl_dev.h (which are not affected) there are no
other usages affecting ABI by spice-gtk, Qemu or spice-server.

Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
Changes since v1:
- update commit message
---
   spice/qxl_dev.h | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spice/qxl_dev.h b/spice/qxl_dev.h
index a9cc4f4..659f930 100644
--- a/spice/qxl_dev.h
+++ b/spice/qxl_dev.h
@@ -275,7 +275,7 @@ typedef struct SPICE_ATTR_ALIGNED(4) SPICE_ATTR_PACKED
QXLRam {
} QXLRam; -typedef union QXLReleaseInfo {
+typedef union SPICE_ATTR_PACKED QXLReleaseInfo {
       uint64_t id;      // in
       uint64_t next;    // out
   } QXLReleaseInfo;
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]