[libvirt] [PATCHv2 1/1] include: function parameter names same in declaration
Christopher Venteicher
cventeic at redhat.com
Tue Feb 13 18:56:59 UTC 2018
----- Original Message -----
> From: "Bjoern Walk" <bwalk at linux.vnet.ibm.com>
> To: "Daniel P. Berrangé" <berrange at redhat.com>
> Cc: "Chris Venteicher" <cventeic at redhat.com>, libvir-list at redhat.com
> Sent: Tuesday, February 13, 2018 5:03:40 AM
> Subject: Re: [libvirt] [PATCHv2 1/1] include: function parameter names same in declaration
>
> Daniel P. Berrangé <berrange at redhat.com> [2018-02-13, 09:47AM +0000]:
> > On Mon, Feb 12, 2018 at 03:24:03PM -0600, Chris Venteicher wrote:
> > > Headers use same function parameter names as definition code.
> > >
> > > In some cases in libvirt-domain and libvirt-network an
> > > established
> > > naming pattern in the header files was more consistent and
> > > informative
> > > in which case the implementation was modified in the c file.
> >
> > > @@ -1626,11 +1626,11 @@ int
> > > virDomainInterfaceStats (virDomainPtr dom,
> > > */
> > > # define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst"
> > >
> > > -int virDomainSetInterfaceParameters
> > > (virDomainPtr dom,
> > > +int virDomainSetInterfaceParameters
> > > (virDomainPtr domain,
> >
> > Hmmmm, I kind of expected that "dom" would be more popular than
> > "domain",
> > but I see the results are somewhat contradictory.
> >
> > If we just consider the header file
> >
> > $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h |
> > wc -l
> > 167
> > $ git grep "virDomainPtr dom" include/libvirt/libvirt-domain.h |
> > grep "virDomainPtr domain" | wc -l
> > 99
> >
> > So dom==68, domain=99 => 2:3
> >
> > But if we consider the source as a whole
> >
> > $ git grep "virDomainPtr dom" | wc -l
> > 1863
> > $ git grep "virDomainPtr dom" | grep "virDomainPtr domain" | wc -l
> > 675
> >
> > So dom=1188 domain=675 => 2:1
>
> This is biased because for a temporary variable an abbreviated name
> 'dom' is preferable, especially if you have an argument 'domain'.
>
> Since function declarations serve some kind of documentation purpose,
> I'd prefer the full name 'domain'. It reads so much better in my
> opinion
> and characters are cheap.
>
A little more background ...
I have only submitted the changes for include/libvirt/*.h so far.
At the bottom is a list of the files impacted by a total of 286 changes suggested by the clang-tidy filter.
The focus here is only the horizontal relationship establishing consistency between definition and declaration,
not the vertical line relationship of consistency between function definitions.
function_definition_1(p1, p2, p3) ---> function_declaration_1(p1,p2,p3)
|
|
|
function_definition_2(p1, p2, p3) ---> function_declaration_2(p1,p2,p3)
|
|
|
function_definition_n(p1, p2, p3) ---> function_declaration_n(p1,p2,p3)
My guess is > 90 percent of the 286 changes make the declaration better.
Probably < 10 percent of the 286 changes make the declaration slightly less readable (ex. domain->dom).
All of the changes make the declarations consistent with the definitions.
None of the changes make the function_definitions worse.
Chris
daemon/remote.c | 2 +-
daemon/stream.h | 2 +-
include/libvirt/libvirt-domain.h | 26 ++++++++++++------------
include/libvirt/libvirt-event.h | 4 ++--
include/libvirt/libvirt-host.h | 4 ++--
include/libvirt/libvirt-interface.h | 4 ++--
include/libvirt/libvirt-network.h | 6 +++---
include/libvirt/libvirt-nwfilter.h | 2 +-
include/libvirt/libvirt-qemu.h | 2 +-
include/libvirt/libvirt-secret.h | 4 ++--
include/libvirt/libvirt-storage.h | 12 ++++++------
include/libvirt/libvirt-stream.h | 22 ++++++++++-----------
src/access/viraccessmanager.c | 2 +-
src/access/viraccessmanager.h | 4 ++--
src/conf/capabilities.c | 2 +-
src/conf/capabilities.h | 2 +-
src/conf/cpu_conf.h | 6 +++---
src/conf/domain_audit.h | 12 ++++++------
src/conf/domain_event.h | 4 ++--
src/conf/networkcommon_conf.h | 4 ++--
src/conf/numa_conf.h | 4 ++--
src/conf/nwfilter_conf.h | 2 +-
src/conf/nwfilter_params.h | 10 +++++-----
src/conf/object_event_private.h | 2 +-
src/conf/secret_conf.h | 2 +-
src/conf/snapshot_conf.h | 4 ++--
src/conf/storage_conf.h | 4 ++--
src/conf/virdomainobjlist.h | 4 ++--
src/conf/virinterfaceobj.c | 2 +-
src/conf/virnetworkobj.c | 4 ++--
src/conf/virnetworkobj.h | 18 ++++++++---------
src/conf/virnodedeviceobj.c | 2 +-
src/conf/virnodedeviceobj.h | 4 ++--
src/conf/virsecretobj.c | 2 +-
src/datatypes.h | 8 ++++----
src/driver.h | 2 +-
src/libvirt_internal.h | 26 ++++++++++++------------
src/locking/lock_manager.h | 10 +++++-----
src/logging/log_handler.h | 2 +-
src/lxc/lxc_monitor.c | 2 +-
src/node_device/node_device_driver.h | 12 ++++++------
src/nwfilter/nwfilter_gentech_driver.h | 4 ++--
src/openvz/openvz_driver.c | 2 +-
src/openvz/openvz_util.h | 2 +-
src/qemu/qemu_agent.h | 6 +++---
src/qemu/qemu_alias.h | 6 +++---
src/qemu/qemu_block.h | 2 +-
src/qemu/qemu_capabilities.h | 4 ++--
src/qemu/qemu_capspriv.h | 2 +-
src/qemu/qemu_cgroup.h | 2 +-
src/qemu/qemu_conf.h | 2 +-
src/qemu/qemu_hotplug.h | 6 +++---
src/qemu/qemu_monitor.h | 8 ++++----
src/qemu/qemu_process.h | 4 ++--
src/rpc/virnetmessage.h | 2 +-
src/rpc/virnetserver.h | 2 +-
src/rpc/virnetserverservice.h | 2 +-
src/rpc/virnetsocket.h | 16 +++++++--------
src/secret/secret_util.h | 4 ++--
src/security/security_dac.h | 2 +-
src/security/security_manager.h | 36 +++++++++++++++++-----------------
src/storage/storage_backend.h | 2 +-
src/uml/uml_conf.h | 2 +-
src/util/viralloc.h | 8 ++++----
src/util/virarch.h | 2 +-
src/util/viraudit.h | 2 +-
src/util/virbitmap.h | 2 +-
src/util/virbuffer.h | 4 ++--
src/util/vircommand.h | 4 ++--
src/util/virerror.h | 4 ++--
src/util/virfile.h | 16 +++++++--------
src/util/virhash.h | 2 +-
src/util/virhostdev.h | 20 +++++++++----------
src/util/viridentity.c | 2 +-
src/util/viriptables.h | 2 +-
src/util/virjson.h | 26 ++++++++++++------------
src/util/virkeycode.h | 2 +-
src/util/virkeyfile.h | 2 +-
src/util/virlog.h | 6 +++---
src/util/virmacaddr.h | 10 +++++-----
src/util/virnetdevbridge.h | 4 ++--
src/util/virnetdevmacvlan.h | 10 +++++-----
src/util/virnetdevvportprofile.h | 4 ++--
src/util/virobject.h | 16 +++++++--------
src/util/virpci.h | 2 +-
src/util/virpidfile.h | 6 +++---
src/util/virqemu.h | 2 +-
src/util/virresctrl.h | 4 ++--
src/util/virsexpr.h | 2 +-
src/util/virsocketaddr.h | 8 ++++----
src/util/virstring.h | 2 +-
src/util/virthreadjob.h | 2 +-
src/util/virthreadpool.h | 2 +-
src/util/virusb.h | 2 +-
src/util/viruuid.h | 4 ++--
src/vbox/vbox_snapshot_conf.h | 4 ++--
src/vmware/vmware_conf.h | 4 ++--
tests/testutils.h | 2 +-
tools/vsh.h | 8 ++++----
99 files changed, 286 insertions(+), 286 deletions(-)
More information about the libvir-list
mailing list