Re: [libvirt] [PATCH v2 06/14] nodedev: Cleanup driver code and prototypes
On 05/26/2017 04:13 AM, Peter Krempa wrote:
On Thu, May 25, 2017 at 15:57:03 -0400, John Ferlan wrote:
> Alter the node_device_driver source and prototypes to follow more
> recent code style guidelines w/r/t spacing between functions, format
> of the function, and the prototype definitions.
>
> While the new names for nodeDeviceUpdateCaps, nodeDeviceUpdateDriverName,
> and nodeDeviceGetTime don't follow exactly w/r/t a "vir" prefix, they
> do follow other driver nomenclature style.
Still, this patch is mixing function renames with whitespace changes.
So add "n" more patches - one to deal with whitespace... one to deal
with function renames... one to deal with argument placement...
I just tried to lump the formatting stuff into one - trying to avoid too
much minutiae. The node_device* code is I would say the oldest and
least touched so it gets the most adjustment - too much.
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/node_device/node_device_driver.c | 41 ++++--
> src/node_device/node_device_driver.h | 93 +++++++++----
> src/node_device/node_device_udev.c | 256 +++++++++++++++++++++--------------
> 3 files changed, 252 insertions(+), 138 deletions(-)
>
> diff --git a/src/node_device/node_device_driver.c
b/src/node_device/node_device_driver.c
> index ba3da62..2a461fb 100644
> --- a/src/node_device/node_device_driver.c
> +++ b/src/node_device/node_device_driver.c
> @@ -151,11 +155,14 @@ void nodeDeviceLock(void)
> {
> virMutexLock(&driver->lock);
> }
> +
> +
> void nodeDeviceUnlock(void)
> {
> virMutexUnlock(&driver->lock);
> }
This one should be fixed too.
[...]
Oh right...
> @@ -478,8 +491,9 @@ nodeDeviceListCaps(virNodeDevicePtr dev, char **const names, int
maxnames)
> return ret;
> }
>
> +
> static int
> -get_time(time_t *t)
> +nodeDeviceGetTime(time_t *t)
> {
> int ret = 0;
>
> @@ -522,7 +536,7 @@ find_new_device(virConnectPtr conn, const char *wwnn, const char
*wwpn)
Why didn't you change name of this function, since you changed the one
above?
Hmm... not sure why. Strange - oh I see it already had the two space
between functions so I skipped right past it.
it's changed to nodeDeviceFindNewDevice in my branch now.
[...]
> diff --git a/src/node_device/node_device_driver.h
b/src/node_device/node_device_driver.h
> index bc8af8a..b46f001 100644
> --- a/src/node_device/node_device_driver.h
> +++ b/src/node_device/node_device_driver.h
> @@ -31,37 +31,75 @@
[...]
> +void
> +nodeDeviceLock(void);
> +
> +void
> +nodeDeviceUnlock(void);
> +
> +extern
> +virNodeDeviceDriverStatePtr driver;
This is ugly. It's also not a function and 'extern' keyword is not the
return type.
Oh right - this got to be such a rote exercise that I overdid it. I
restored it back to one line.
[...]
ACK if you fix nodeDeviceUnlock too. Also please don't mix the header
whitespace update with function renaming in future patches.
Well the horse has already left the barn on that front for some on list
patches... But I can go back through those patches that haven't been
posted yet and create singular change.
Tks -
John