Re: [libvirt] [PATCH 2/3] conf: Clean up nodedev code

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