[libvirt] [PATCH 2/3] conf: Clean up nodedev code
Daniel P. Berrange
berrange at redhat.com
Thu Mar 2 09:30:23 UTC 2017
On Thu, Mar 02, 2017 at 10:04:24AM +0100, Michal Privoznik wrote:
> On 03/02/2017 09:58 AM, Peter Krempa wrote:
> > On Wed, Mar 01, 2017 at 19:27:15 -0500, John Ferlan wrote:
> >> Alter the static functions from virNodeDev* to just nodeDev* as a visual
> >> cue to determine which are local or not when reading code.
> >>
> >> Cleanup spacing between functions, function defs, etc. to match more modern
> >> techniques used in libvirt
> >>
> >> Signed-off-by: John Ferlan <jferlan at redhat.com>
> >> ---
> >> src/conf/node_device_conf.c | 476 +++++++++++++++++++++++---------------------
> >> src/conf/virnodedeviceobj.c | 128 ++++++------
> >> 2 files changed, 322 insertions(+), 282 deletions(-)
> >>
> >> diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
> >> index bc36527..09e815a 100644
> >> --- a/src/conf/node_device_conf.c
> >> +++ b/src/conf/node_device_conf.c
> >> @@ -72,9 +72,9 @@ VIR_ENUM_IMPL(virNodeDevDRM, VIR_NODE_DEV_DRM_LAST,
> >> "render")
> >>
> >> static int
> >> -virNodeDevCapsDefParseString(const char *xpath,
> >> - xmlXPathContextPtr ctxt,
> >> - char **string)
> >> +nodeDevCapsDefParseString(const char *xpath,
> >> + xmlXPathContextPtr ctxt,
> >> + char **string)
> >
> > Please don't remove the vir prefix. The coding style tries to converge
> > to having them everywhere.
> >
>
> Why? If a function is static, we can be sure it's not called from
> outside of the file. Moreover, I'd direct your attention to recent
> commit of f557b3351e0b6d for instance. In fact whole qemu driver serves
> as a great example: it's "static int qemuDomain*()" not "static vir
> virQEMUDomain*()".
>
> In fact, I'd suggest the opposite rule - use "vir" prefix only if
> function is shared between modules. For instance virFileCopyACLs should
> have the vir prefix because it's exported. virFileRewriteStrHelper
> should not have the prefix because it's static.
> The advantage of this approach is that one can immediately tell just
> from the name if the function is exported or not.
No, this is a bad rule because it causes us to rename code when we
inevitably make static functions non-static. We want a naming rule
that is standardized & stable long term. Convering to use 'vir'
prefix everywhere gives us that.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
More information about the libvir-list
mailing list