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

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

 



On 04/23/2013 04:26 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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

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