[libvirt] [PATCH 00/22] syntax-check: Check misaligned stuff and fix them (batch II)

Shi Lei posted 22 patches 5 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20181005081145.29901-1-shi_lei@massclouds.com
build-aux/check-spacing.pl                 | 247 +++++++++++----------
src/access/viraccessdriverpolkit.c         |   6 +-
src/admin/admin_server.c                   |   2 +-
src/admin/admin_server_dispatch.c          |   4 +-
src/bhyve/bhyve_capabilities.c             |   7 +-
src/bhyve/bhyve_command.c                  |   6 +-
src/bhyve/bhyve_driver.c                   |  10 +-
src/bhyve/bhyve_monitor.c                  |   4 +-
src/bhyve/bhyve_parse_command.c            |  16 +-
src/bhyve/bhyve_process.c                  |   6 +-
src/cpu/cpu_ppc64.c                        |   4 +-
src/cpu/cpu_x86.c                          |  16 +-
src/interface/interface_backend_udev.c     | 102 +++++----
src/libvirt-domain.c                       |  14 +-
src/libvirt-host.c                         |   4 +-
src/libvirt-lxc.c                          |   5 +-
src/libvirt-network.c                      |   2 +-
src/libvirt-nodedev.c                      |   4 +-
src/libvirt-storage.c                      |  13 +-
src/libvirt.c                              |   4 +-
src/locking/lock_manager.c                 |   4 +-
src/network/bridge_driver.c                |  25 +--
src/nwfilter/nwfilter_dhcpsnoop.c          |  10 +-
src/nwfilter/nwfilter_driver.c             |   8 +-
src/nwfilter/nwfilter_ebiptables_driver.c  |  18 +-
src/nwfilter/nwfilter_learnipaddr.c        |  36 ++-
src/phyp/phyp_driver.c                     |  20 +-
src/remote/remote_daemon.c                 |   2 +-
src/remote/remote_daemon_dispatch.c        |  16 +-
src/remote/remote_driver.c                 |  31 +--
src/rpc/virnetclient.c                     |  40 ++--
src/rpc/virnetlibsshsession.c              |   2 +-
src/rpc/virnetservermdns.c                 |   4 +-
src/rpc/virnetsocket.c                     |  23 +-
src/rpc/virnetsshsession.c                 |   2 +-
src/storage/storage_backend_disk.c         |   7 +-
src/storage/storage_backend_gluster.c      |   2 +-
src/storage/storage_backend_iscsi_direct.c |   2 +-
src/storage/storage_backend_rbd.c          |  14 +-
src/storage/storage_driver.c               |   6 +-
src/storage/storage_util.c                 |  16 +-
src/uml/uml_driver.c                       |  50 ++---
src/vmware/vmware_conf.c                   |   2 +-
src/vmware/vmware_driver.c                 |  10 +-
src/vmx/vmx.c                              |  23 +-
src/vz/vz_driver.c                         |  14 +-
src/vz/vz_sdk.c                            |  26 ++-
src/vz/vz_utils.c                          |   2 +-
src/xenapi/xenapi_utils.c                  |  12 +-
src/xenconfig/xen_common.c                 |  21 +-
src/xenconfig/xen_xl.c                     |  23 +-
src/xenconfig/xen_xm.c                     |   8 +-
52 files changed, 488 insertions(+), 467 deletions(-)
[libvirt] [PATCH 00/22] syntax-check: Check misaligned stuff and fix them (batch II)
Posted by Shi Lei 5 years, 5 months ago
This series is batch(II) and it continues to fix misaligned arguments and misaligned
conditions in src directory. The batch I:
https://www.redhat.com/archives/libvir-list/2018-September/msg00888.html
In addition to fix misaligned stuff, this series includes several other jobs:

==========================================
1. Relax the checking-rule for misaligned.
==========================================
In addition to the check-rule in batch I, a new code style is okay:
Linefeed can be put behind the open-parenthesis of the function,
the position of argument should be:
 1) pos % 4 == 0
 2) pos > start_of_function_name(includes func's parent_pointer)
 3) pos is the first place which matches 1) and 2)

E.g.

[indent]if (virLongLongLongFunctionName(
[4-spaces-align]LongLongLongFirstName,
[4-spaces-align]LongLongLongSecondName) < 0)

or

[indent]ret = pointer->virFunctionName(
[4-spaces-align]LongLongLongFirstName,
[4-spaces-align]LongLongLongSecondName);

====================================================
2. Add indicator to help correcting misaligned stuff.
====================================================
The [patch 21/22] definitely indicates the correct position.

===========================================
3. Improve the speed of check-spacing.pl .
===========================================
The [patch 22/22] uses global var, rather than passing arguments
to subroutine. It can be a bit faster.

The benchmark for check-spacing.pl on my PC:

(before batch I  )    1.6s
(before batch II )    3.3s
(apply this patch)    2.7s

====================================================
# Remaining work after this batch. (for src directory)
====================================================
DIR        Misaligned(lines)
esx            334
qemu           227
hyperv         104
lxc            60
conf           76
libxl          69
vbox           65
test           60
security       62

Plan to fix them in two or three weeks.


Shi Lei (22):
  access: Fix misaligned arguments
  admin: Fix misaligned arguments
  bhyve: Fix misaligned arguments and misaligned conditions
  cpu: Fix misaligned arguments
  interface: Fix misaligned arguments
  locking: Fix misaligned arguments
  network: Fix misaligned arguments
  src:libvirt*.c: Fix misaligned arguments
  nwfilter: Fix misaligned arguments and misaligned conditions
  phyp: Fix misaligned arguments
  remote: Fix misaligned arguments
  rpc: Fix misaligned arguments
  storage: Fix misaligned arguments and misaligned conditions
  uml: Fix misaligned arguments
  vmware: Fix misaligned arguments and misaligned conditions
  vmx: Fix misaligned arguments
  vz: Fix misaligned arguments and misaligned conditions
  xenapi: Fix misaligned arguments
  xenconfig: Fix misaligned arguments and misaligned conditions
  build-aux:check-spacing: Relax the check-rule for misaligned stuff
  build-aux:check-spacing: Add indicator to help correcting misaligned
    stuff
  build-aux:check-spacing: Remove arguments of subroutines for speed

 build-aux/check-spacing.pl                 | 247 +++++++++++----------
 src/access/viraccessdriverpolkit.c         |   6 +-
 src/admin/admin_server.c                   |   2 +-
 src/admin/admin_server_dispatch.c          |   4 +-
 src/bhyve/bhyve_capabilities.c             |   7 +-
 src/bhyve/bhyve_command.c                  |   6 +-
 src/bhyve/bhyve_driver.c                   |  10 +-
 src/bhyve/bhyve_monitor.c                  |   4 +-
 src/bhyve/bhyve_parse_command.c            |  16 +-
 src/bhyve/bhyve_process.c                  |   6 +-
 src/cpu/cpu_ppc64.c                        |   4 +-
 src/cpu/cpu_x86.c                          |  16 +-
 src/interface/interface_backend_udev.c     | 102 +++++----
 src/libvirt-domain.c                       |  14 +-
 src/libvirt-host.c                         |   4 +-
 src/libvirt-lxc.c                          |   5 +-
 src/libvirt-network.c                      |   2 +-
 src/libvirt-nodedev.c                      |   4 +-
 src/libvirt-storage.c                      |  13 +-
 src/libvirt.c                              |   4 +-
 src/locking/lock_manager.c                 |   4 +-
 src/network/bridge_driver.c                |  25 +--
 src/nwfilter/nwfilter_dhcpsnoop.c          |  10 +-
 src/nwfilter/nwfilter_driver.c             |   8 +-
 src/nwfilter/nwfilter_ebiptables_driver.c  |  18 +-
 src/nwfilter/nwfilter_learnipaddr.c        |  36 ++-
 src/phyp/phyp_driver.c                     |  20 +-
 src/remote/remote_daemon.c                 |   2 +-
 src/remote/remote_daemon_dispatch.c        |  16 +-
 src/remote/remote_driver.c                 |  31 +--
 src/rpc/virnetclient.c                     |  40 ++--
 src/rpc/virnetlibsshsession.c              |   2 +-
 src/rpc/virnetservermdns.c                 |   4 +-
 src/rpc/virnetsocket.c                     |  23 +-
 src/rpc/virnetsshsession.c                 |   2 +-
 src/storage/storage_backend_disk.c         |   7 +-
 src/storage/storage_backend_gluster.c      |   2 +-
 src/storage/storage_backend_iscsi_direct.c |   2 +-
 src/storage/storage_backend_rbd.c          |  14 +-
 src/storage/storage_driver.c               |   6 +-
 src/storage/storage_util.c                 |  16 +-
 src/uml/uml_driver.c                       |  50 ++---
 src/vmware/vmware_conf.c                   |   2 +-
 src/vmware/vmware_driver.c                 |  10 +-
 src/vmx/vmx.c                              |  23 +-
 src/vz/vz_driver.c                         |  14 +-
 src/vz/vz_sdk.c                            |  26 ++-
 src/vz/vz_utils.c                          |   2 +-
 src/xenapi/xenapi_utils.c                  |  12 +-
 src/xenconfig/xen_common.c                 |  21 +-
 src/xenconfig/xen_xl.c                     |  23 +-
 src/xenconfig/xen_xm.c                     |   8 +-
 52 files changed, 488 insertions(+), 467 deletions(-)

-- 
2.17.1


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/22] syntax-check: Check misaligned stuff and fix them (batch II)
Posted by Daniel P. Berrangé 5 years, 5 months ago
On Fri, Oct 05, 2018 at 04:11:23PM +0800, Shi Lei wrote:
> This series is batch(II) and it continues to fix misaligned arguments and misaligned
> conditions in src directory. The batch I:
> https://www.redhat.com/archives/libvir-list/2018-September/msg00888.html
> In addition to fix misaligned stuff, this series includes several other jobs:

I was away when that series merged, but I'm not at all a fan of writing
our own custom code for checking indentation.

IMHO we should be just picking an existing tool such as either ident
or clang-format, and providing a desired config for it.

I've looked at clang-format before and it is pretty good. It does not
have ability to quite match our code style, but I don't think that
is a real show stopper. We'd be better off just adapting to one of
the common code styles rather than inventing yet another app todo
code indent analysis.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/22] syntax-check: Check misaligned stuff and fix them (batch II)
Posted by Andrea Bolognani 5 years, 5 months ago
On Fri, 2018-10-05 at 09:31 +0100, Daniel P. Berrangé wrote:
> I've looked at clang-format before and it is pretty good. It does not
> have ability to quite match our code style, but I don't think that
> is a real show stopper. We'd be better off just adapting to one of
> the common code styles rather than inventing yet another app todo
> code indent analysis.

I *strongly* support this idea :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/22] syntax-check: Check misaligned stuff and fix them (batch II)
Posted by Daniel P. Berrangé 5 years, 5 months ago
On Fri, Oct 05, 2018 at 11:11:28AM +0200, Andrea Bolognani wrote:
> On Fri, 2018-10-05 at 09:31 +0100, Daniel P. Berrangé wrote:
> > I've looked at clang-format before and it is pretty good. It does not
> > have ability to quite match our code style, but I don't think that
> > is a real show stopper. We'd be better off just adapting to one of
> > the common code styles rather than inventing yet another app todo
> > code indent analysis.
> 
> I *strongly* support this idea :)

The other benefit of clang-format is that it can actually fix the code
automatically, as well as reporting on the problem. Our syntax check
tool merely reports the problem leaving the developer tedious work to
fix each problem.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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