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

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



On 04/03/2013 09:06 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> 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 redhat com>
> ---
>  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


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