Re: [PATCH 3/7] Use VIR_ALLOC_VAR instead of VIR_ALLOC_N for creating virObject

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

 



On 04/03/2013 09:06 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> The current way virObject instances are allocated using
> VIR_ALLOC_N causes alignment warnings
> 
> util/virobject.c: In function 'virObjectNew':
> util/virobject.c:195:11: error: cast increases required alignment of target type [-Werror=cast-align]
> 
> Changing to use VIR_ALLOC_VAR will avoid the need todo

s/todo/to do/

> the casts entirely.
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  src/util/virobject.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virobject.c b/src/util/virobject.c
> index 808edc4..93e37e4 100644
> --- a/src/util/virobject.c
> +++ b/src/util/virobject.c
> @@ -186,13 +186,13 @@ bool virClassIsDerivedFrom(virClassPtr klass,
>  void *virObjectNew(virClassPtr klass)
>  {
>      virObjectPtr obj = NULL;
> -    char *somebytes;
>  
> -    if (VIR_ALLOC_N(somebytes, klass->objectSize) < 0) {

If klass->objectSize was not a multiple of sizeof(void*), then the
compiler warning is correct.  I seriously doubt any of the callers had
this issue, though, since klass->objectSize was determined by sizeof() a
struct that included virObject as a base class, and virObject includes a
void* member (virClassPtr klass).  So the warning is spurious.

> +    if (VIR_ALLOC_VAR(obj,
> +                      char,
> +                      klass->objectSize - sizeof(virObject)) < 0) {

However, I agree that this formulation avoids the issue.

The only concern I have is if a child class ever includes a 'long
double' on a platform where the use of long double introduces stronger
alignment requirements than 'void *'.  But since we don't use 'long
double' (except for virtTestCountAverage() in the testsuite, and even
then I think that usage is overkill), it's academic.  And even if we
did, we could increase the alignment of virObject by having it contain a
union with a long double member, even without increasing its size
(although it might get tricky for how to do that without breaking
compilation of existing code).  Let's not worry about it for today.

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]