[libvirt] [PATCH v2 3/5] virobject: Introduce VIR_CLASS_NEW() macro

[Daniel P. Berrangé]

On Tue, Apr 17, 2018 at 10:20:51AM +0200, Michal Privoznik wrote:
> So far we are repeating the following lines over and over:
> 
>   if (!(virSomeObjectClass = virClassNew(virClassForObject(),
>                              "virSomeObject",
>                              sizeof(virSomeObject),
>                              virSomeObjectDispose);
>       return -1;
> 
> While this works, it is impossible to do some checking. Firstly,
> the class name (the 2nd argument) doesn't match the name in the
> code in all cases (the 3rd argument). Secondly, the current style
> is needlessly verbose. This commit turns example into following:
> 
>   VIR_CLASS_NEW(virClassForObject(),
>                 virSomeObject);
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>

[snip]

> diff --git a/src/access/viraccessmanager.c b/src/access/viraccessmanager.c
> index c268ec57f7..a8d361d389 100644
> --- a/src/access/viraccessmanager.c
> +++ b/src/access/viraccessmanager.c
> @@ -54,11 +54,8 @@ static void virAccessManagerDispose(void *obj);
>  
>  static int virAccessManagerOnceInit(void)
>  {
> -    if (!(virAccessManagerClass = virClassNew(virClassForObjectLockable(),
> -                                              "virAccessManagerClass",
> -                                              sizeof(virAccessManager),
> -                                              virAccessManagerDispose)))
> -        return -1;
> +    VIR_CLASS_NEW(virClassForObjectLockable(),
> +                  virAccessManager);

Ewww, I definitely do not like this approach - it is hiding control
flow which can exit the callpath inside a macro which is a big no.
It isn't hard to make it work in an explicit way as

   if (VIR_CLASS_NEW(virClassForObjectLockable(),
                     virAccessManager) < 0)
      return -1;

I'd also suggest we swap around the parameter order - have the
type name being allocated as the first method.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|