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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux