[libvirt PATCH 0/1] DO NOT MERGE: RFC: targetted usage of clang-format

Daniel P. Berrangé posted 1 patch 2 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20221115105910.256814-1-berrange@redhat.com
.clang-format                    | 32 +++++++++++++++++++++++++
mark.pl                          | 41 ++++++++++++++++++++++++++++++++
src/util/glibcompat.c            |  2 ++
src/util/glibcompat.h            |  4 +++-
src/util/iohelper.c              | 10 ++++----
src/util/viralloc.c              |  5 +++-
src/util/viralloc.h              |  2 ++
src/util/virarch.c               |  4 +++-
src/util/virarch.h               |  2 ++
src/util/virarptable.c           |  2 ++
src/util/virarptable.h           |  2 ++
src/util/viraudit.c              |  6 +++--
src/util/viraudit.h              |  2 ++
src/util/virauth.c               | 13 ++++++----
src/util/virauth.h               |  2 ++
src/util/virauthconfig.c         |  6 +++--
src/util/virauthconfig.h         |  2 ++
src/util/virbitmap.c             |  5 +++-
src/util/virbitmap.h             |  5 ++--
src/util/virbpf.c                |  5 +++-
src/util/virbpf.h                |  2 ++
src/util/virbuffer.c             |  2 ++
src/util/virbuffer.h             |  3 ++-
src/util/virccw.c                |  4 ++++
src/util/virccw.h                |  2 ++
src/util/vircgroup.c             | 15 ++++++------
src/util/vircgroup.h             |  2 ++
src/util/vircgroupbackend.c      |  2 ++
src/util/vircgroupbackend.h      |  3 ++-
src/util/vircgrouppriv.h         |  2 ++
src/util/vircgroupv1.c           | 11 +++++----
src/util/vircgroupv1.h           |  2 ++
src/util/vircgroupv2.c           |  5 ++--
src/util/vircgroupv2.h           |  2 ++
src/util/vircgroupv2devices.c    |  5 ++--
src/util/vircgroupv2devices.h    |  3 ++-
src/util/vircommand.c            | 10 ++++----
src/util/vircommand.h            |  2 ++
src/util/vircommandpriv.h        |  2 ++
src/util/virconf.c               | 20 +++++++++-------
src/util/virconf.h               |  3 +++
src/util/vircrypto.c             |  9 ++++---
src/util/vircrypto.h             |  2 ++
src/util/virdaemon.c             | 14 ++++++-----
src/util/virdaemon.h             |  2 ++
src/util/virdevmapper.c          | 15 +++++++-----
src/util/virdevmapper.h          |  2 ++
src/util/virdnsmasq.c            | 17 +++++++------
src/util/virdnsmasq.h            |  2 ++
src/util/virebtables.c           |  7 ++++--
src/util/virebtables.h           |  2 ++
src/util/virendian.h             |  3 +++
src/util/virenum.c               |  2 ++
src/util/virenum.h               |  2 ++
src/util/virerror.c              |  5 +++-
src/util/virerror.h              |  2 ++
src/util/virerrorpriv.h          |  2 ++
src/util/virevent.c              |  5 +++-
src/util/virevent.h              |  2 ++
src/util/vireventglib.c          |  5 +++-
src/util/vireventglib.h          |  2 ++
src/util/vireventglibwatch.c     |  3 +++
src/util/vireventglibwatch.h     |  2 ++
src/util/vireventthread.c        |  5 +++-
src/util/vireventthread.h        |  5 +++-
src/util/virfcp.c                |  7 +++---
src/util/virfcp.h                |  2 ++
src/util/virfdstream.c           | 19 ++++++++-------
src/util/virfdstream.h           |  3 +++
src/util/virfile.c               | 13 ++++++----
src/util/virfile.h               |  2 ++
src/util/virfilecache.c          | 13 +++++-----
src/util/virfilecache.h          |  5 ++--
src/util/virfirewall.c           |  9 ++++---
src/util/virfirewall.h           |  4 +++-
src/util/virfirewalld.c          | 11 +++++----
src/util/virfirewalld.h          |  2 ++
src/util/virfirewalldpriv.h      |  2 ++
src/util/virfirmware.c           |  5 +++-
src/util/virfirmware.h           |  2 ++
src/util/virgdbus.c              |  4 +++-
src/util/virgdbus.h              |  2 ++
src/util/virgettext.c            |  4 +++-
src/util/virgettext.h            |  2 ++
src/util/virgic.c                |  6 ++++-
src/util/virgic.h                |  2 ++
src/util/virglibutil.c           |  2 ++
src/util/virglibutil.h           |  2 ++
src/util/virhash.c               |  8 ++++---
src/util/virhash.h               |  2 ++
src/util/virhashcode.c           |  5 +++-
src/util/virhashcode.h           |  5 +++-
src/util/virhook.c               | 16 ++++++++-----
src/util/virhook.h               |  2 ++
src/util/virhostcpu.c            | 12 ++++++----
src/util/virhostcpu.h            |  1 +
src/util/virhostcpupriv.h        |  2 ++
src/util/virhostmem.c            | 14 ++++++-----
src/util/virhostmem.h            |  2 ++
src/util/virhostuptime.c         |  8 ++++---
src/util/virhostuptime.h         |  2 ++
src/util/viridentity.c           | 13 ++++++----
src/util/viridentity.h           |  5 +++-
src/util/viridentitypriv.h       |  2 ++
src/util/virinitctl.c            |  7 ++++--
src/util/virinitctl.h            |  2 ++
src/util/viriptables.c           | 15 +++++++-----
src/util/viriptables.h           |  4 +++-
src/util/viriscsi.c              |  2 ++
src/util/viriscsi.h              |  2 ++
src/util/virjson.c               | 10 ++++----
src/util/virjson.h               |  5 ++--
src/util/virkeycode.c            |  6 ++++-
src/util/virkeycode.h            |  2 ++
src/util/virkmod.c               |  6 ++++-
src/util/virkmod.h               |  2 ++
src/util/virlease.c              |  8 ++++---
src/util/virlease.h              |  2 ++
src/util/virlockspace.c          | 13 ++++++----
src/util/virlockspace.h          |  2 ++
src/util/virlog.c                | 24 +++++++++++--------
src/util/virlog.h                |  2 ++
src/util/virmacaddr.c            |  4 +++-
src/util/virmacaddr.h            |  2 ++
src/util/virmacmap.c             |  9 ++++---
src/util/virmacmap.h             |  2 ++
src/util/virmdev.c               |  7 ++++--
src/util/virmdev.h               |  4 +++-
src/util/virmodule.c             |  5 +++-
src/util/virmodule.h             |  2 ++
src/util/virnetdev.c             | 24 +++++++++++--------
src/util/virnetdev.h             |  9 ++++---
src/util/virnetdevbandwidth.c    |  6 ++++-
src/util/virnetdevbandwidth.h    |  2 ++
src/util/virnetdevbridge.c       | 15 +++++++-----
src/util/virnetdevbridge.h       |  2 ++
src/util/virnetdevip.c           | 15 +++++++-----
src/util/virnetdevip.h           |  2 ++
src/util/virnetdevmacvlan.c      |  5 +++-
src/util/virnetdevmacvlan.h      |  6 +++--
src/util/virnetdevmidonet.c      |  3 +++
src/util/virnetdevmidonet.h      |  1 +
src/util/virnetdevopenvswitch.c  | 12 ++++++----
src/util/virnetdevopenvswitch.h  |  4 +++-
src/util/virnetdevpriv.h         |  2 ++
src/util/virnetdevtap.c          | 22 ++++++++++-------
src/util/virnetdevtap.h          |  4 +++-
src/util/virnetdevveth.c         |  5 +++-
src/util/virnetdevveth.h         |  2 ++
src/util/virnetdevvlan.c         |  5 +++-
src/util/virnetdevvlan.h         |  2 ++
src/util/virnetdevvportprofile.c |  5 +++-
src/util/virnetdevvportprofile.h |  6 +++--
src/util/virnetlink.c            | 11 +++++----
src/util/virnetlink.h            |  2 ++
src/util/virnodesuspend.c        | 12 ++++++----
src/util/virnodesuspend.h        |  2 ++
src/util/virnuma.c               | 14 ++++++-----
src/util/virnuma.h               |  1 +
src/util/virnvme.c               |  7 ++++--
src/util/virnvme.h               |  2 ++
src/util/virobject.c             |  6 +++--
src/util/virobject.h             |  4 +++-
src/util/virpci.c                | 16 +++++++------
src/util/virpci.h                |  4 +++-
src/util/virpcivpd.c             |  8 ++++---
src/util/virpcivpd.h             |  2 ++
src/util/virpcivpdpriv.h         |  2 ++
src/util/virperf.c               |  6 +++--
src/util/virperf.h               |  2 ++
src/util/virpidfile.c            | 11 +++++----
src/util/virpidfile.h            |  3 +++
src/util/virpolkit.c             | 10 +++++---
src/util/virpolkit.h             |  2 ++
src/util/virportallocator.c      |  9 ++++---
src/util/virportallocator.h      |  2 ++
src/util/virprobe.h              |  2 ++
src/util/virprocess.c            | 13 +++++-----
src/util/virprocess.h            |  2 ++
src/util/virqemu.c               |  8 ++++---
src/util/virqemu.h               |  2 ++
src/util/virrandom.c             | 11 +++++----
src/util/virrandom.h             |  2 ++
src/util/virresctrl.c            |  8 ++++---
src/util/virresctrl.h            |  5 ++--
src/util/virresctrlpriv.h        |  2 ++
src/util/virrotatingfile.c       |  3 +++
src/util/virrotatingfile.h       |  2 ++
src/util/virscsi.c               | 11 +++++----
src/util/virscsi.h               |  2 ++
src/util/virscsihost.c           |  6 ++++-
src/util/virscsihost.h           |  2 ++
src/util/virscsivhost.c          |  8 +++++--
src/util/virscsivhost.h          |  2 ++
src/util/virseclabel.c           |  5 +++-
src/util/virseclabel.h           |  2 ++
src/util/virsecret.c             |  5 +++-
src/util/virsecret.h             |  5 ++--
src/util/virsecureerase.c        |  2 ++
src/util/virsecureerase.h        |  2 ++
src/util/virsocket.c             |  9 ++++---
src/util/virsocket.h             |  2 ++
src/util/virsocketaddr.c         |  5 +++-
src/util/virsocketaddr.h         |  2 ++
src/util/virstoragefile.c        |  5 +++-
src/util/virstoragefile.h        |  2 ++
src/util/virstring.c             |  6 +++--
src/util/virstring.h             |  2 ++
src/util/virsysinfo.c            |  9 ++++---
src/util/virsysinfo.h            |  2 ++
src/util/virsysinfopriv.h        |  2 ++
src/util/virsystemd.c            | 17 ++++++-------
src/util/virsystemd.h            |  2 ++
src/util/virsystemdpriv.h        |  2 ++
src/util/virthread.c             |  3 ++-
src/util/virthread.h             |  4 +++-
src/util/virthreadjob.c          |  5 +++-
src/util/virthreadjob.h          |  2 ++
src/util/virthreadpool.c         |  5 +++-
src/util/virthreadpool.h         |  2 ++
src/util/virtime.c               |  5 +++-
src/util/virtime.h               |  2 ++
src/util/virtpm.c                | 11 +++++----
src/util/virtpm.h                |  2 ++
src/util/virtypedparam-public.c  |  4 +++-
src/util/virtypedparam.c         |  5 +++-
src/util/virtypedparam.h         |  2 ++
src/util/viruri.c                |  4 +++-
src/util/viruri.h                |  2 ++
src/util/virusb.c                |  9 ++++---
src/util/virusb.h                |  2 ++
src/util/virutil.c               | 14 ++++++-----
src/util/virutil.h               |  6 +++--
src/util/viruuid.c               | 10 ++++----
src/util/viruuid.h               |  1 +
src/util/virvhba.c               |  5 +++-
src/util/virvhba.h               |  2 ++
src/util/virvsock.c              |  4 ++--
src/util/virvsock.h              |  2 ++
src/util/virxdrdefs.h            |  2 ++
src/util/virxml.c                | 13 ++++++----
src/util/virxml.h                |  7 +++---
242 files changed, 986 insertions(+), 367 deletions(-)
create mode 100644 .clang-format
create mode 100644 mark.pl
[libvirt PATCH 0/1] DO NOT MERGE: RFC: targetted usage of clang-format
Posted by Daniel P. Berrangé 2 weeks, 5 days ago
While we have a code style, it is not perfectly applied across the code
base because its impossible for humans to manage that without using
automated tooling. clang-format is the closest we'll get to a code
formatter we could use, but still it would reformat quite alot of our
code.

I discovered that '/* clang-format off */' can be used to stop it from
reformatting sections of code. It is not practical to add that comment
around all places we don't want touched. I thought we could perhaps
use it as a way to limit clang-format to merely do sorting & regrouping
of #include statements though.

This change illustrates what that would look like for the src/util
directory, so we can consider whuether it is worth it.

I've included a mark.pl script that I usd to auto-add the magic
comment. It gets it wrong sometimes, so needs inspection. If we
did decide to use this, we would need the magic comment in every
existing source file.

Then there is the question about new source files. If a contributor
forgets to add the comment, then entire new source file will be
processed by clang-format. This might be desirable. If so, we will
need to fully expand the .clang-format config file to match ou4r
desired style. Right now I only recorded include file rules.

Or we could just wrong a dumb script to do #include sorting
ourselves and carry on ignoring clang-format.

I'm pretty undecided myself.

Daniel P. Berrangé (1):
  DO NOT MERGE: use clang-format for sorting #include statement

 .clang-format                    | 32 +++++++++++++++++++++++++
 mark.pl                          | 41 ++++++++++++++++++++++++++++++++
 src/util/glibcompat.c            |  2 ++
 src/util/glibcompat.h            |  4 +++-
 src/util/iohelper.c              | 10 ++++----
 src/util/viralloc.c              |  5 +++-
 src/util/viralloc.h              |  2 ++
 src/util/virarch.c               |  4 +++-
 src/util/virarch.h               |  2 ++
 src/util/virarptable.c           |  2 ++
 src/util/virarptable.h           |  2 ++
 src/util/viraudit.c              |  6 +++--
 src/util/viraudit.h              |  2 ++
 src/util/virauth.c               | 13 ++++++----
 src/util/virauth.h               |  2 ++
 src/util/virauthconfig.c         |  6 +++--
 src/util/virauthconfig.h         |  2 ++
 src/util/virbitmap.c             |  5 +++-
 src/util/virbitmap.h             |  5 ++--
 src/util/virbpf.c                |  5 +++-
 src/util/virbpf.h                |  2 ++
 src/util/virbuffer.c             |  2 ++
 src/util/virbuffer.h             |  3 ++-
 src/util/virccw.c                |  4 ++++
 src/util/virccw.h                |  2 ++
 src/util/vircgroup.c             | 15 ++++++------
 src/util/vircgroup.h             |  2 ++
 src/util/vircgroupbackend.c      |  2 ++
 src/util/vircgroupbackend.h      |  3 ++-
 src/util/vircgrouppriv.h         |  2 ++
 src/util/vircgroupv1.c           | 11 +++++----
 src/util/vircgroupv1.h           |  2 ++
 src/util/vircgroupv2.c           |  5 ++--
 src/util/vircgroupv2.h           |  2 ++
 src/util/vircgroupv2devices.c    |  5 ++--
 src/util/vircgroupv2devices.h    |  3 ++-
 src/util/vircommand.c            | 10 ++++----
 src/util/vircommand.h            |  2 ++
 src/util/vircommandpriv.h        |  2 ++
 src/util/virconf.c               | 20 +++++++++-------
 src/util/virconf.h               |  3 +++
 src/util/vircrypto.c             |  9 ++++---
 src/util/vircrypto.h             |  2 ++
 src/util/virdaemon.c             | 14 ++++++-----
 src/util/virdaemon.h             |  2 ++
 src/util/virdevmapper.c          | 15 +++++++-----
 src/util/virdevmapper.h          |  2 ++
 src/util/virdnsmasq.c            | 17 +++++++------
 src/util/virdnsmasq.h            |  2 ++
 src/util/virebtables.c           |  7 ++++--
 src/util/virebtables.h           |  2 ++
 src/util/virendian.h             |  3 +++
 src/util/virenum.c               |  2 ++
 src/util/virenum.h               |  2 ++
 src/util/virerror.c              |  5 +++-
 src/util/virerror.h              |  2 ++
 src/util/virerrorpriv.h          |  2 ++
 src/util/virevent.c              |  5 +++-
 src/util/virevent.h              |  2 ++
 src/util/vireventglib.c          |  5 +++-
 src/util/vireventglib.h          |  2 ++
 src/util/vireventglibwatch.c     |  3 +++
 src/util/vireventglibwatch.h     |  2 ++
 src/util/vireventthread.c        |  5 +++-
 src/util/vireventthread.h        |  5 +++-
 src/util/virfcp.c                |  7 +++---
 src/util/virfcp.h                |  2 ++
 src/util/virfdstream.c           | 19 ++++++++-------
 src/util/virfdstream.h           |  3 +++
 src/util/virfile.c               | 13 ++++++----
 src/util/virfile.h               |  2 ++
 src/util/virfilecache.c          | 13 +++++-----
 src/util/virfilecache.h          |  5 ++--
 src/util/virfirewall.c           |  9 ++++---
 src/util/virfirewall.h           |  4 +++-
 src/util/virfirewalld.c          | 11 +++++----
 src/util/virfirewalld.h          |  2 ++
 src/util/virfirewalldpriv.h      |  2 ++
 src/util/virfirmware.c           |  5 +++-
 src/util/virfirmware.h           |  2 ++
 src/util/virgdbus.c              |  4 +++-
 src/util/virgdbus.h              |  2 ++
 src/util/virgettext.c            |  4 +++-
 src/util/virgettext.h            |  2 ++
 src/util/virgic.c                |  6 ++++-
 src/util/virgic.h                |  2 ++
 src/util/virglibutil.c           |  2 ++
 src/util/virglibutil.h           |  2 ++
 src/util/virhash.c               |  8 ++++---
 src/util/virhash.h               |  2 ++
 src/util/virhashcode.c           |  5 +++-
 src/util/virhashcode.h           |  5 +++-
 src/util/virhook.c               | 16 ++++++++-----
 src/util/virhook.h               |  2 ++
 src/util/virhostcpu.c            | 12 ++++++----
 src/util/virhostcpu.h            |  1 +
 src/util/virhostcpupriv.h        |  2 ++
 src/util/virhostmem.c            | 14 ++++++-----
 src/util/virhostmem.h            |  2 ++
 src/util/virhostuptime.c         |  8 ++++---
 src/util/virhostuptime.h         |  2 ++
 src/util/viridentity.c           | 13 ++++++----
 src/util/viridentity.h           |  5 +++-
 src/util/viridentitypriv.h       |  2 ++
 src/util/virinitctl.c            |  7 ++++--
 src/util/virinitctl.h            |  2 ++
 src/util/viriptables.c           | 15 +++++++-----
 src/util/viriptables.h           |  4 +++-
 src/util/viriscsi.c              |  2 ++
 src/util/viriscsi.h              |  2 ++
 src/util/virjson.c               | 10 ++++----
 src/util/virjson.h               |  5 ++--
 src/util/virkeycode.c            |  6 ++++-
 src/util/virkeycode.h            |  2 ++
 src/util/virkmod.c               |  6 ++++-
 src/util/virkmod.h               |  2 ++
 src/util/virlease.c              |  8 ++++---
 src/util/virlease.h              |  2 ++
 src/util/virlockspace.c          | 13 ++++++----
 src/util/virlockspace.h          |  2 ++
 src/util/virlog.c                | 24 +++++++++++--------
 src/util/virlog.h                |  2 ++
 src/util/virmacaddr.c            |  4 +++-
 src/util/virmacaddr.h            |  2 ++
 src/util/virmacmap.c             |  9 ++++---
 src/util/virmacmap.h             |  2 ++
 src/util/virmdev.c               |  7 ++++--
 src/util/virmdev.h               |  4 +++-
 src/util/virmodule.c             |  5 +++-
 src/util/virmodule.h             |  2 ++
 src/util/virnetdev.c             | 24 +++++++++++--------
 src/util/virnetdev.h             |  9 ++++---
 src/util/virnetdevbandwidth.c    |  6 ++++-
 src/util/virnetdevbandwidth.h    |  2 ++
 src/util/virnetdevbridge.c       | 15 +++++++-----
 src/util/virnetdevbridge.h       |  2 ++
 src/util/virnetdevip.c           | 15 +++++++-----
 src/util/virnetdevip.h           |  2 ++
 src/util/virnetdevmacvlan.c      |  5 +++-
 src/util/virnetdevmacvlan.h      |  6 +++--
 src/util/virnetdevmidonet.c      |  3 +++
 src/util/virnetdevmidonet.h      |  1 +
 src/util/virnetdevopenvswitch.c  | 12 ++++++----
 src/util/virnetdevopenvswitch.h  |  4 +++-
 src/util/virnetdevpriv.h         |  2 ++
 src/util/virnetdevtap.c          | 22 ++++++++++-------
 src/util/virnetdevtap.h          |  4 +++-
 src/util/virnetdevveth.c         |  5 +++-
 src/util/virnetdevveth.h         |  2 ++
 src/util/virnetdevvlan.c         |  5 +++-
 src/util/virnetdevvlan.h         |  2 ++
 src/util/virnetdevvportprofile.c |  5 +++-
 src/util/virnetdevvportprofile.h |  6 +++--
 src/util/virnetlink.c            | 11 +++++----
 src/util/virnetlink.h            |  2 ++
 src/util/virnodesuspend.c        | 12 ++++++----
 src/util/virnodesuspend.h        |  2 ++
 src/util/virnuma.c               | 14 ++++++-----
 src/util/virnuma.h               |  1 +
 src/util/virnvme.c               |  7 ++++--
 src/util/virnvme.h               |  2 ++
 src/util/virobject.c             |  6 +++--
 src/util/virobject.h             |  4 +++-
 src/util/virpci.c                | 16 +++++++------
 src/util/virpci.h                |  4 +++-
 src/util/virpcivpd.c             |  8 ++++---
 src/util/virpcivpd.h             |  2 ++
 src/util/virpcivpdpriv.h         |  2 ++
 src/util/virperf.c               |  6 +++--
 src/util/virperf.h               |  2 ++
 src/util/virpidfile.c            | 11 +++++----
 src/util/virpidfile.h            |  3 +++
 src/util/virpolkit.c             | 10 +++++---
 src/util/virpolkit.h             |  2 ++
 src/util/virportallocator.c      |  9 ++++---
 src/util/virportallocator.h      |  2 ++
 src/util/virprobe.h              |  2 ++
 src/util/virprocess.c            | 13 +++++-----
 src/util/virprocess.h            |  2 ++
 src/util/virqemu.c               |  8 ++++---
 src/util/virqemu.h               |  2 ++
 src/util/virrandom.c             | 11 +++++----
 src/util/virrandom.h             |  2 ++
 src/util/virresctrl.c            |  8 ++++---
 src/util/virresctrl.h            |  5 ++--
 src/util/virresctrlpriv.h        |  2 ++
 src/util/virrotatingfile.c       |  3 +++
 src/util/virrotatingfile.h       |  2 ++
 src/util/virscsi.c               | 11 +++++----
 src/util/virscsi.h               |  2 ++
 src/util/virscsihost.c           |  6 ++++-
 src/util/virscsihost.h           |  2 ++
 src/util/virscsivhost.c          |  8 +++++--
 src/util/virscsivhost.h          |  2 ++
 src/util/virseclabel.c           |  5 +++-
 src/util/virseclabel.h           |  2 ++
 src/util/virsecret.c             |  5 +++-
 src/util/virsecret.h             |  5 ++--
 src/util/virsecureerase.c        |  2 ++
 src/util/virsecureerase.h        |  2 ++
 src/util/virsocket.c             |  9 ++++---
 src/util/virsocket.h             |  2 ++
 src/util/virsocketaddr.c         |  5 +++-
 src/util/virsocketaddr.h         |  2 ++
 src/util/virstoragefile.c        |  5 +++-
 src/util/virstoragefile.h        |  2 ++
 src/util/virstring.c             |  6 +++--
 src/util/virstring.h             |  2 ++
 src/util/virsysinfo.c            |  9 ++++---
 src/util/virsysinfo.h            |  2 ++
 src/util/virsysinfopriv.h        |  2 ++
 src/util/virsystemd.c            | 17 ++++++-------
 src/util/virsystemd.h            |  2 ++
 src/util/virsystemdpriv.h        |  2 ++
 src/util/virthread.c             |  3 ++-
 src/util/virthread.h             |  4 +++-
 src/util/virthreadjob.c          |  5 +++-
 src/util/virthreadjob.h          |  2 ++
 src/util/virthreadpool.c         |  5 +++-
 src/util/virthreadpool.h         |  2 ++
 src/util/virtime.c               |  5 +++-
 src/util/virtime.h               |  2 ++
 src/util/virtpm.c                | 11 +++++----
 src/util/virtpm.h                |  2 ++
 src/util/virtypedparam-public.c  |  4 +++-
 src/util/virtypedparam.c         |  5 +++-
 src/util/virtypedparam.h         |  2 ++
 src/util/viruri.c                |  4 +++-
 src/util/viruri.h                |  2 ++
 src/util/virusb.c                |  9 ++++---
 src/util/virusb.h                |  2 ++
 src/util/virutil.c               | 14 ++++++-----
 src/util/virutil.h               |  6 +++--
 src/util/viruuid.c               | 10 ++++----
 src/util/viruuid.h               |  1 +
 src/util/virvhba.c               |  5 +++-
 src/util/virvhba.h               |  2 ++
 src/util/virvsock.c              |  4 ++--
 src/util/virvsock.h              |  2 ++
 src/util/virxdrdefs.h            |  2 ++
 src/util/virxml.c                | 13 ++++++----
 src/util/virxml.h                |  7 +++---
 242 files changed, 986 insertions(+), 367 deletions(-)
 create mode 100644 .clang-format
 create mode 100644 mark.pl

-- 
2.37.3

Re: [libvirt PATCH 0/1] DO NOT MERGE: RFC: targetted usage of clang-format
Posted by Michal Prívozník 1 week, 6 days ago
On 11/15/22 11:59, Daniel P. Berrangé wrote:
> While we have a code style, it is not perfectly applied across the code
> base because its impossible for humans to manage that without using
> automated tooling. clang-format is the closest we'll get to a code
> formatter we could use, but still it would reformat quite alot of our
> code.
> 
> I discovered that '/* clang-format off */' can be used to stop it from
> reformatting sections of code. It is not practical to add that comment
> around all places we don't want touched. I thought we could perhaps
> use it as a way to limit clang-format to merely do sorting & regrouping
> of #include statements though.
> 
> This change illustrates what that would look like for the src/util
> directory, so we can consider whuether it is worth it.
> 
> I've included a mark.pl script that I usd to auto-add the magic
> comment. It gets it wrong sometimes, so needs inspection. If we
> did decide to use this, we would need the magic comment in every
> existing source file.
> 
> Then there is the question about new source files. If a contributor
> forgets to add the comment, then entire new source file will be
> processed by clang-format. This might be desirable. If so, we will
> need to fully expand the .clang-format config file to match ou4r
> desired style. Right now I only recorded include file rules.
> 
> Or we could just wrong a dumb script to do #include sorting
> ourselves and carry on ignoring clang-format.
> 
> I'm pretty undecided myself.


I think unsorted #includes are the least offending part. And in fact, I
think mixing their order may hurt our portability. Manpages for some
functions list two includes (though I can't find anything right now).
And not always in alphabetical order. Although, at that point one can
argue if they are order sensitive then it's bug in header files themselves.

IMO we should use an automatic formatting tool. Of course use a
configuration that's as close to our current style as possible. I'm
somewhat reconciled that it will not be 100% match, but if we come up
with good enough .vimrc then I'm good.

Subsequently we ought to make our syntax-check rule required test
(!expensive) and enforce formatting for benefit of new contributors.
They seem to struggle with our somewhat exotic style as I point it out
in my reviews a lot lately.

Michal

Re: [libvirt PATCH 0/1] DO NOT MERGE: RFC: targetted usage of clang-format
Posted by Martin Kletzander 2 weeks, 5 days ago
On Tue, Nov 15, 2022 at 10:59:09AM +0000, Daniel P. Berrangé wrote:
>While we have a code style, it is not perfectly applied across the code
>base because its impossible for humans to manage that without using
>automated tooling. clang-format is the closest we'll get to a code
>formatter we could use, but still it would reformat quite alot of our
>code.
>
>I discovered that '/* clang-format off */' can be used to stop it from
>reformatting sections of code. It is not practical to add that comment
>around all places we don't want touched. I thought we could perhaps
>use it as a way to limit clang-format to merely do sorting & regrouping
>of #include statements though.
>
>This change illustrates what that would look like for the src/util
>directory, so we can consider whuether it is worth it.
>
>I've included a mark.pl script that I usd to auto-add the magic
>comment. It gets it wrong sometimes, so needs inspection. If we
>did decide to use this, we would need the magic comment in every
>existing source file.
>
>Then there is the question about new source files. If a contributor
>forgets to add the comment, then entire new source file will be
>processed by clang-format. This might be desirable. If so, we will
>need to fully expand the .clang-format config file to match ou4r
>desired style. Right now I only recorded include file rules.
>
>Or we could just wrong a dumb script to do #include sorting
>ourselves and carry on ignoring clang-format.
>
>I'm pretty undecided myself.
>

Few of my personal comments below, beware that they are very subjective.
They relate to different paragraphs above, so I just dumped them
altogether.

Lately I am becoming more and more appreciative of code formatters.  The
consistency is certainly appealing when reading and understanding the
.code flow.

However, unless there are no false positives we cannot depend on it 100%
of the time (e.g. in CI or other checks) and it stops being used by
contributors and reviewers.  So picking one thing that it does might be
okay as a starting point, but having all the extra stuff in the source
feels messy.  Especially when not all files are covered the same way.

And I do not think a tool needs to completely adapt to our style, just
like I do not blindly expect others to follow my style.  Consistency
brings more to the table than each person trying to push for their
preferred way of formatting.  It can be beneficial for the people to
adjust to the tool.  Well, in a reasonable manner of course.

I would much rather prefer adjusting to a tool that covers all parts
of the formatting than using just a part of what a tool does and adjust
the code and tool configuration for just one (unimportant) thing.

Sorted includes are not something I feel is helpful when reviewing the
code, for example.  It could be much cleaner if we, for example, had a
global util.h and conf.h which would include util/*.h and conf/*.h,
respectively, in a correct order, as that would make it a bit more
error-proof, even though I don't consider that particularly nice either.

So I, personally, do not like messing with the code, adding duplicated
comments and scripts.  Instead I am willing to sacrifice some of the
things we are used to, decide on a configuration file for a formatter
that covers every piece of indentation and formatting (e.g. does not
skip some parts of the code because it does not understand them, even
though there would be changes it might take some time getting used to),
and then ideally then toss all related syntax-checks, but have
consistent code format that we can check for in CI and locally.  Not to
mention that most editors, especially when combined with an LSP, already
support using the formatters when saving a file, for example.

Anyway, those are my €.02, thanks for reading the whole *subjective* essay =)

Martin