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

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(a)redhat.com&gt; [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;