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

Re: [libvirt] [PATCH 4/6] Extend previous check to validate driver struct field names



On 04/23/2013 04:26 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> Ensure that the driver struct field names match the public
> API names. For an API virXXXX we must have a driver struct
> field xXXXX. ie strip the leading 'vir' and lowercase any
> leading uppercase letters.
> 
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  daemon/remote.c                         |   2 +-
...
>  tests/qemuxml2argvtest.c                |  22 +-
>  55 files changed, 1205 insertions(+), 1197 deletions(-)

Big, but mostly mechanical.

If you use git format-patch -O/path/to/file, you can format your patch
to rearrange particular globs to come first; so that the meat of the
patch comes before the mechanical fallout.

> +++ b/src/check-drivername.pl
> @@ -45,6 +45,19 @@ while (<DRVFILE>) {
>          my $name = $1;
>          print "Bogus name $1\n";
>          $status = 1;
> +    } elsif (/^\s*(virDrv\w+)\s+(\w+);\s*/) {
> +        my $drv = $1;
> +        my $field = $2;
> +
> +        my $tmp = $drv;
> +        $tmp =~ s/virDrv//;
> +        $tmp =~ s/^NWFilter/nwfilter/;
> +        $tmp =~ s/^(\w)/lc $1/e;
> +
> +        unless ($tmp eq $field) {
> +            print "Driver struct field $field should be named $tmp\n";
> +            $status = 1;
> +        }
>      }

Thankfully, it already fell pretty near the beginning.

> +++ b/src/driver.h
> @@ -63,8 +63,8 @@ typedef enum {
>   *   0     Feature is not supported.
>   */
>  # define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature)                         \
> -    ((drv)->supports_feature ?                                              \
> -        (drv)->supports_feature((conn), (feature)) > 0 : 0)
> +    ((drv)->connectSupportsFeature ?                                              \
> +        (drv)->connectSupportsFeature((conn), (feature)) > 0 : 0)

Worth re-aligning the \ on this one?

> +++ b/src/esx/esx_device_monitor.c
> @@ -69,8 +69,8 @@ esxDeviceClose(virConnectPtr conn)
>  
>  static virDeviceMonitor esxDeviceMonitor = {
>      .name = "ESX",
> -    .open = esxDeviceOpen, /* 0.7.6 */
> -    .close = esxDeviceClose, /* 0.7.6 */
> +    .connectOpen = esxDeviceOpen, /* 0.7.6 */
> +    .connectClose = esxDeviceClose, /* 0.7.6 */

Not this patch, but should any followup patches rename driver functions
to more recognizable names?

> +++ b/src/libvirt_internal.h
> @@ -113,7 +113,7 @@ enum {
>  };
>  
>  
> -int virDrvSupportsFeature (virConnectPtr conn, int feature);
> +int virConnectSupportsFeature (virConnectPtr conn, int feature);

Worth dropping the space before ( on this line while touching it?

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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