On Tue, Apr 17, 2018 at 05:07:41PM +0200, Michal Privoznik wrote:
On 04/17/2018 10:32 AM, Daniel P. Berrangé wrote:
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@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;
So if VIR_CLASS_NEW() should wrap virClassNew() how come this example compares the result with integer? Shouldn't hat be:
if (!VIR_CLAS_NEW(..)) return -1;
Yes, my bad - I had VIR_ALLOC() on the brain when i mistakenly wrote < 0 instead of == NULL (or just !).
or do you see VIR_CLASS_NEW defined as an expression returning integer, e.g. like this:
# define VIR_CLASS_NEW(name, prnt) \ ((name##Class = virClassNew(prnt, #name, sizeof(name), name##Dispose)) ? 0 : -1)
No, it was a mistake. 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 :|