On Thu, Mar 02, 2017 at 10:35:57AM +0100, Michal Privoznik wrote: > On 03/02/2017 10:30 AM, Daniel P. Berrange wrote: > > 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@xxxxxxxxxx> > >>>> --- > >>>> 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. > > I fail to see why would renaming be a problem. Going from static to > non-static would require to change all the callers in just one file > (which is not going to have much callers anyway). Going the opposite > direction - well, there are no callers in other files anyway. If there > were we couldn't make the funtion static. Even though all the code may be in one file, it can still create a huge amount of code churn. eg if we make qemuDomObjFromDomain non-static, that'd involve updating 125 lines in the qemu_driver.c file. That's alot of self-inflicted pain for backporting patches that come afterwards. 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/ :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
