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

Re: [libvirt] [PATCH v2 06/14] nodedev: Cleanup driver code and prototypes



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.

> Signed-off-by: John Ferlan <jferlan 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.

[...]

> @@ -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?

[...]

> 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.

[...]

ACK if you fix nodeDeviceUnlock too. Also please don't mix the header
whitespace update with function renaming in future patches.

Attachment: signature.asc
Description: PGP signature


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