Re: [libvirt] [PATCH] object: require maximal alignment in base class

Recent changes to events (commit 8a29ffcf) resulted in new compile failures on some targets (such as ARM OMAP5): conf/domain_event.c: In function 'virDomainEventDispatchDefaultFunc': conf/domain_event.c:1198:30: error: cast increases required alignment of target type [-Werror=cast-align] conf/domain_event.c:1314:34: error: cast increases required alignment of target type [-Werror=cast-align] cc1: all warnings being treated as errors The error is due to alignment; the base class is merely aligned to the worst of 'int' and 'void*', while the child class must be aligned to a 'long long'. The solution is to include a 'long long' (and for good measure, a function pointer) in the base class to ensure correct alignment regardless of what a child class may add, but to wrap the inclusion in a union so as to not incur any wasted space. On a typical x86_64 platform, the base class remains 16 bytes; on i686, the base class remains 12 bytes; and on the impacted ARM platform, the base class grows from 12 bytes to 16 bytes due to the increase of alignment from 4 to 8 bytes. Reported by Michele Paolino and others. * src/util/virobject.h (_virObject): Use a union to ensure that subclasses never have stricter alignment than the parent. * src/util/virobject.c (virObjectNew, virObjectUnref) (virObjectRef): Adjust clients. * src/libvirt.c (virConnectRef, virDomainRef, virNetworkRef) (virInterfaceRef, virStoragePoolRef, virStorageVolRef) (virNodeDeviceRef, virSecretRef, virStreamRef, virNWFilterRef) (virDomainSnapshotRef): Likewise. * src/qemu/qemu_monitor.c (qemuMonitorOpenInternal) (qemuMonitorClose): Likewise. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- Even though this fixes a build-breaker, I'd rather that someone actually experiencing the build failure test that this fixes the problem for them before I push (I don't have easy access to hardware exhibiting the problem). src/libvirt.c | 22 +++++++++++----------- src/qemu/qemu_monitor.c | 4 ++-- src/util/virobject.c | 10 +++++----- src/util/virobject.h | 16 ++++++++++++++-- 4 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/util/virobject.h b/src/util/virobject.h index 3a08f10..d571b5c 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -36,9 +36,21 @@ typedef virObjectLockable *virObjectLockablePtr; typedef void (*virObjectDisposeCallback)(void *obj); +/* Most code should not play with the contents of this struct; however, + * the struct itself is public so that it can be embedded as the first + * field of a subclassed object. */ struct _virObject { - unsigned int magic; - int refs; + /* Ensure correct alignment of this and all subclasses, even on + * platforms where 'long long' or function pointers have stricter + * requirements than 'void *'. */ + union { + long long dummy_align1; + void (*dummy_align2) (void); + struct { + unsigned int magic; + int refs; + } s; + } u; virClassPtr klass; };