[libvirt] [PATCH 7/9] virobject: Introduce VIR_CLASS_NEW() macro

Michal Privoznik mprivozn at redhat.com
Mon Apr 16 16:27:47 UTC 2018


On 04/16/2018 11:52 AM, Erik Skultety wrote:
> On Fri, Apr 13, 2018 at 04:47:14PM +0200, Michal Privoznik wrote:
>> So far we are repeating the following lines over and over:
>>
>>   virClassNew(virClassForObject(),
>>               "virSomeObject",
>>               sizeof(virSomeObject),
>>               virSomeObjectDispose);
>>
>> 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>
>> ---
>>  src/access/viraccessmanager.c           |   6 +-
>>  src/bhyve/bhyve_conf.c                  |   6 +-
>>  src/conf/capabilities.c                 |   6 +-
>>  src/conf/domain_capabilities.c          |  12 +--
>>  src/conf/domain_conf.c                  |  18 ++---
>>  src/conf/domain_event.c                 | 126 +++++++++++---------------------
>>  src/conf/network_event.c                |  12 +--
>>  src/conf/node_device_event.c            |  18 ++---
>>  src/conf/object_event.c                 |  12 +--
>>  src/conf/secret_event.c                 |  18 ++---
>>  src/conf/storage_event.c                |  18 ++---
>>  src/conf/virdomainobjlist.c             |   6 +-
>>  src/conf/virinterfaceobj.c              |  12 +--
>>  src/conf/virnetworkobj.c                |  12 +--
>>  src/conf/virnodedeviceobj.c             |  12 +--
>>  src/conf/virsecretobj.c                 |  12 +--
>>  src/conf/virstorageobj.c                |  24 ++----
>>  src/datatypes.c                         |   6 +-
>>  src/interface/interface_backend_netcf.c |   6 +-
>>  src/libvirt-admin.c                     |   6 +-
>>  src/libxl/libxl_conf.c                  |   6 +-
>>  src/libxl/libxl_domain.c                |   6 +-
>>  src/libxl/libxl_migration.c             |   6 +-
>>  src/logging/log_handler.c               |   6 +-
>>  src/lxc/lxc_conf.c                      |   6 +-
>>  src/lxc/lxc_monitor.c                   |   6 +-
>>  src/node_device/node_device_udev.c      |   6 +-
>>  src/qemu/qemu_agent.c                   |   6 +-
>>  src/qemu/qemu_capabilities.c            |   6 +-
>>  src/qemu/qemu_conf.c                    |   6 +-
>>  src/qemu/qemu_domain.c                  |  36 +++------
>>  src/qemu/qemu_monitor.c                 |   6 +-
>>  src/rpc/virkeepalive.c                  |   6 +-
>>  src/rpc/virnetclient.c                  |   6 +-
>>  src/rpc/virnetclientprogram.c           |   6 +-
>>  src/rpc/virnetclientstream.c            |   6 +-
>>  src/rpc/virnetdaemon.c                  |   6 +-
>>  src/rpc/virnetlibsshsession.c           |   6 +-
>>  src/rpc/virnetsaslcontext.c             |  12 +--
>>  src/rpc/virnetserver.c                  |   6 +-
>>  src/rpc/virnetserverclient.c            |   6 +-
>>  src/rpc/virnetserverprogram.c           |   6 +-
>>  src/rpc/virnetserverservice.c           |   6 +-
>>  src/rpc/virnetsocket.c                  |   6 +-
>>  src/rpc/virnetsshsession.c              |   6 +-
>>  src/rpc/virnettlscontext.c              |  12 +--
>>  src/security/security_manager.c         |   6 +-
>>  src/util/virclosecallbacks.c            |   6 +-
>>  src/util/virdnsmasq.c                   |   7 +-
>>  src/util/virfdstream.c                  |   6 +-
>>  src/util/virfilecache.c                 |   6 +-
>>  src/util/virhash.c                      |   6 +-
>>  src/util/virhostdev.c                   |   6 +-
>>  src/util/viridentity.c                  |   6 +-
>>  src/util/virmacmap.c                    |   6 +-
>>  src/util/virmdev.c                      |   6 +-
>>  src/util/virobject.c                    |  12 +--
>>  src/util/virobject.h                    |   4 +
>>  src/util/virpci.c                       |   6 +-
>>  src/util/virportallocator.c             |   6 +-
>>  src/util/virresctrl.c                   |  12 +--
>>  src/util/virscsi.c                      |   6 +-
>>  src/util/virscsivhost.c                 |   6 +-
>>  src/util/virusb.c                       |   6 +-
>>  src/vbox/vbox_common.c                  |   6 +-
>>  src/vz/vz_driver.c                      |   6 +-
>>  tests/virfilecachetest.c                |   6 +-
> 
> ...
> 
>> diff --git a/src/datatypes.c b/src/datatypes.c
>> index 0c3c66a9ce..3016e45076 100644
>> --- a/src/datatypes.c
>> +++ b/src/datatypes.c
>> @@ -74,10 +74,8 @@ static int
>>  virDataTypesOnceInit(void)
>>  {
>>  #define DECLARE_CLASS_COMMON(basename, parent) \
>> -    if (!(basename ## Class = virClassNew(parent, \
>> -                                          #basename, \
>> -                                          sizeof(basename), \
>> -                                          basename ## Dispose))) \
>> +    if (!(basename ## Class = VIR_CLASS_NEW(parent, \
>> +                                            basename))) \
>>          return -1;
> 
> I'd probably go one step further and move the DECLARE_CLASS macros in a widely
> accessible header and use that instead of repeating the following:
> 
> if (!(virSomethingClass = VIR_CLASS_NEW(virObject(Lockable)?, virSomething))
>     return -1;

Well, it's not that simple. There are multiple classes new ones are
derived from. We will still need two arguments to the macro. But it can
help us unify class variable names. Okay, I'll cook patch like that.

Michal




More information about the libvir-list mailing list