[Spice-devel] [PATCH spice-protocol 2/3] qxl_dev: Fix alignment for QXLReleaseInfo

[Uri Lublin]

On 5/13/19 12:45 PM, Frediano Ziglio wrote:
> Do not declare the structure as aligned.
> The start/end-packed.h headers affects only MingW or Microsoft
> compilers.

They also define/undef SPICE_ATTR_PACKED which affects Linux.

BTW, if we define PACKED as ALIGNED(1) does that work for Windows
too (I mean instead of pragma pack) ?

> To have unaligned structure with GCC compiler you have
> to use SPICE_ATTR_PACKED. 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.
> As this structure has no holes this change does not make any size
> change using any compiler.

For this reason, this change looks safe to me.

> 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 as such by spice-gtk, Qemu or spice-server.

It is used in Qemu, in hw/display/qxl.h, struct PCIQXLDevice.
Also it's used in spice-server and likely QXL drivers too.

Uri.

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>   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;
>