This series takes a good swing at two annoyances:

* We sometimes use #include "..." even for system headers, and <...>
  for our own headers.  Makes spotting the system headers harder, and
  can be confusing.  PATCH 01 cleans this up.

* Our use of header guards is rather sloppy.  Sloppiness there can
  lead to confusing compilation errors.  The rest of the series cleans
  up existing header guards.  In particular, it normalizes guard
  symbols to follow a common pattern, in the hope of making clashes
  less likely.  It doesn't add new header guards.  We have more than
  200 headers without a recognizable header guard.  A few of them are
  for multiple inclusion, a few more don't need header guards because
  they don't do anything but include, but the majority probably should
  have one.  Left for another day.

I cleaned this up in 2016 (merge commit ca3d87d4c84), but as it turns
out, we're pretty good at adding annoyances.

Markus Armbruster (8):
  Use #include "..." for our own headers, <...> for others
  authz: Normalize #include "authz/trace.h" to "trace.h"
  linux-user/nios2 linux-user/riscv: Clean up header guards
  target/xtensa: Clean up core-isa.h header guards
  Clean up header guards that don't match their file name
  Clean up ill-advised or unusual header guards
  Normalize header guard symbol definition.
  Clean up decorations and whitespace around header guards

 authz/base.c                                   |  2 +-
 authz/list.c                                   |  2 +-
 authz/listfile.c                               |  2 +-
 authz/pamacct.c                                |  2 +-
 authz/simple.c                                 |  2 +-
 block/crypto.h                                 |  6 +++---
 contrib/elf2dmp/qemu_elf.h                     |  6 +++---
 contrib/rdmacm-mux/main.c                      | 18 +++++++++---------
 contrib/rdmacm-mux/rdmacm-mux.h                |  6 +++---
 disas/nanomips.h                               |  4 ++--
 fsdev/qemu-fsdev-throttle.h                    |  7 ++++---
 hw/arm/smmuv3-internal.h                       |  4 ++--
 hw/display/vga_regs.h                          |  6 +++---
 hw/i386/amd_iommu.h                            |  4 ++--
 hw/ide/ahci_internal.h                         |  2 +-
 hw/rdma/rdma_utils.h                           |  1 -
 hw/rdma/vmw/pvrdma_qp_ops.h                    |  4 ++--
 hw/sd/sdmmc-internal.h                         |  5 +++--
 hw/timer/m48t59-internal.h                     |  3 ++-
 hw/tpm/tpm_ioctl.h                             |  7 ++++---
 hw/xtensa/xtensa_memory.h                      |  4 ++--
 include/authz/base.h                           |  7 +++----
 include/authz/list.h                           |  7 +++----
 include/authz/listfile.h                       |  8 +++-----
 include/authz/pamacct.h                        |  7 +++----
 include/authz/simple.h                         |  7 +++----
 include/block/aio-wait.h                       |  2 +-
 include/chardev/spice.h                        |  4 ++--
 include/disas/capstone.h                       |  2 +-
 include/exec/translator.h                      |  2 +-
 include/hw/arm/nrf51_soc.h                     |  1 -
 include/hw/arm/smmu-common.h                   |  2 +-
 include/hw/audio/soundhw.h                     |  4 ++--
 include/hw/i386/x86-iommu.h                    |  4 ++--
 include/hw/intc/heathrow_pic.h                 |  6 +++---
 include/hw/intc/xlnx-pmu-iomod-intc.h          |  6 +++---
 include/hw/misc/armsse-mhu.h                   |  4 ++--
 include/hw/misc/imx2_wdt.h                     |  2 +-
 include/hw/misc/nrf51_rng.h                    |  3 ++-
 include/hw/pci-host/designware.h               |  2 +-
 include/hw/pci-host/sabre.h                    |  4 ++--
 include/hw/ppc/pnv.h                           |  7 ++++---
 include/hw/ppc/pnv_core.h                      |  7 ++++---
 include/hw/ppc/pnv_lpc.h                       |  7 ++++---
 include/hw/ppc/pnv_occ.h                       |  7 ++++---
 include/hw/ppc/pnv_psi.h                       |  7 ++++---
 include/hw/ppc/pnv_xscom.h                     |  7 ++++---
 include/hw/ppc/spapr_ovec.h                    |  7 ++++---
 include/hw/riscv/sifive_plic.h                 |  1 -
 include/hw/scsi/emulation.h                    |  2 +-
 include/hw/timer/pl031.h                       |  4 ++--
 include/hw/virtio/vhost-vsock.h                |  6 +++---
 include/hw/virtio/virtio-crypto.h              |  6 +++---
 include/hw/watchdog/wdt_aspeed.h               |  7 ++++---
 include/hw/xen/start_info.h                    |  6 +++---
 include/hw/xen/xen-legacy-backend.h            |  6 +++---
 include/hw/xtensa/mx_pic.h                     |  4 ++--
 include/hw/xtensa/xtensa-isa.h                 |  6 +++---
 include/migration/qemu-file-types.h            |  4 ++--
 include/qemu/drm.h                             |  4 ++--
 include/qemu/filemonitor.h                     |  6 +++---
 include/qemu/jhash.h                           |  6 +++---
 include/qemu/pmem.h                            |  2 +-
 include/qemu/stats64.h                         |  2 +-
 include/qemu/sys_membarrier.h                  |  2 +-
 include/qemu/systemd.h                         |  2 +-
 include/scsi/constants.h                       |  4 ++--
 include/scsi/utils.h                           |  2 +-
 include/sysemu/hvf.h                           |  5 +++--
 include/ui/kbd-state.h                         |  3 ++-
 linux-user/nios2/target_cpu.h                  |  4 ++--
 linux-user/nios2/target_signal.h               |  6 +++---
 linux-user/nios2/target_structs.h              |  4 ++--
 linux-user/nios2/target_syscall.h              |  6 +++---
 linux-user/riscv/target_cpu.h                  |  4 ++--
 linux-user/riscv/target_signal.h               |  6 +++---
 linux-user/riscv/target_structs.h              |  4 ++--
 linux-user/xtensa/syscall_nr.h                 |  6 +++---
 linux-user/xtensa/target_structs.h             |  4 ++--
 linux-user/xtensa/termbits.h                   |  6 +++---
 net/colo.h                                     |  6 +++---
 qga/vss-win32/vss-handles.h                    |  4 ++--
 scsi/pr-helper.h                               |  3 ++-
 slirp/src/debug.h                              |  6 +++---
 slirp/src/stream.h                             |  6 +++---
 slirp/src/util.h                               |  5 +++--
 slirp/src/vmstate.h                            |  5 +++--
 target/i386/hax-i386.h                         |  4 ++--
 target/i386/hax-interface.h                    |  4 ++--
 target/i386/hax-posix.h                        |  6 +++---
 target/i386/hvf/hvf-i386.h                     |  4 ++--
 target/i386/hvf/vmcs.h                         |  4 ++--
 target/i386/hvf/x86.h                          |  2 +-
 target/i386/hvf/x86_decode.h                   |  2 +-
 target/i386/hvf/x86_descr.h                    |  2 +-
 target/i386/hvf/x86_emu.h                      |  5 +++--
 target/i386/hvf/x86_flags.h                    |  7 ++++---
 target/i386/hvf/x86_mmu.h                      |  7 ++++---
 target/i386/hvf/x86_task.h                     |  6 ++++--
 target/i386/whp-dispatch.h                     |  2 +-
 target/i386/whpx-all.c                         |  1 -
 target/nios2/cpu.h                             |  7 ++++---
 target/nios2/mmu.h                             |  7 ++++---
 target/ppc/mmu-book3s-v3.h                     |  6 +++---
 target/riscv/pmp.h                             |  4 ++--
 target/sparc/asi.h                             |  6 +++---
 target/xtensa/core-de212/core-isa.h            |  8 +++-----
 .../xtensa/core-sample_controller/core-isa.h   |  8 +++-----
 target/xtensa/core-test_kc705_be/core-isa.h    |  8 +++-----
 target/xtensa/core-test_mmuhifi_c3/core-isa.h  |  8 +++-----
 target/xtensa/xtensa-isa-internal.h            |  2 +-
 tests/acpi-utils.h                             |  2 +-
 tests/libqos/e1000e.h                          |  4 ++--
 tests/libqos/qgraph_internal.h                 |  4 ++--
 tests/libqos/sdhci.h                           |  4 ++--
 tests/migration/migration-test.h               |  7 ++++---
 tests/tpm-emu.h                                |  2 +-
 117 files changed, 278 insertions(+), 270 deletions(-)

-- 
2.17.2

On Fri, Mar 15, 2019 at 03:51:15PM +0100, Markus Armbruster wrote:
> * Our use of header guards is rather sloppy.  Sloppiness there can
>   lead to confusing compilation errors.  The rest of the series cleans
>   up existing header guards.  In particular, it normalizes guard
>   symbols to follow a common pattern, in the hope of making clashes
>   less likely.  It doesn't add new header guards.  We have more than
>   200 headers without a recognizable header guard.  A few of them are
>   for multiple inclusion, a few more don't need header guards because
>   they don't do anything but include, but the majority probably should
>   have one.  Left for another day.

Time for #pragma once?
https://en.wikipedia.org/wiki/Pragma_once

Stefan

On Wed, Mar 20, 2019 at 01:52:26PM +0000, Stefan Hajnoczi wrote:
> On Fri, Mar 15, 2019 at 03:51:15PM +0100, Markus Armbruster wrote:
> > * Our use of header guards is rather sloppy.  Sloppiness there can
> >   lead to confusing compilation errors.  The rest of the series cleans
> >   up existing header guards.  In particular, it normalizes guard
> >   symbols to follow a common pattern, in the hope of making clashes
> >   less likely.  It doesn't add new header guards.  We have more than
> >   200 headers without a recognizable header guard.  A few of them are
> >   for multiple inclusion, a few more don't need header guards because
> >   they don't do anything but include, but the majority probably should
> >   have one.  Left for another day.
> 
> Time for #pragma once?
> https://en.wikipedia.org/wiki/Pragma_once

That seems like a reasonable idea to me. We already mandate CLang or
GCC as only supported compilers, so there's no portability concerns.


On the subject of non-standard C extensions, I was also wondering if
QEMU should consider use of __attribute__((cleanup($func))) for doing
automatically free'ing of memory when variables go out of scope. As
well as reducing possibility of memory leaks, this simplifies code
as it often allows for many "goto cleanup" jumps to be replaced with
direct "return -1"s.  It isn't the most attractive syntax, but that
downside doesn't outweigh the positives from my POV.

We recently started using it in libvirt for this reason.

Glib2 itself already uses this since 2.44 and exposes some macros for
client code to use for this.

We bumped min glib to 2.40 a year go because Debian Jessie has 2.42
and Ubuntu Trusty needs 2.40 which Peter used as a build merge test
machine.

Our platform support policy will let us drop Jessie support when
Buster is released fairly soon. If Peter no longer uses Trusty
machines for buld, we'll soon be able to have a min glib that uses
attribute(cleanup) as a standard dep.

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

On Wed, Mar 20, 2019 at 02:54:22PM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 20, 2019 at 01:52:26PM +0000, Stefan Hajnoczi wrote:
> > On Fri, Mar 15, 2019 at 03:51:15PM +0100, Markus Armbruster wrote:
> > > * Our use of header guards is rather sloppy.  Sloppiness there can
> > >   lead to confusing compilation errors.  The rest of the series cleans
> > >   up existing header guards.  In particular, it normalizes guard
> > >   symbols to follow a common pattern, in the hope of making clashes
> > >   less likely.  It doesn't add new header guards.  We have more than
> > >   200 headers without a recognizable header guard.  A few of them are
> > >   for multiple inclusion, a few more don't need header guards because
> > >   they don't do anything but include, but the majority probably should
> > >   have one.  Left for another day.
> > 
> > Time for #pragma once?
> > https://en.wikipedia.org/wiki/Pragma_once
> 
> That seems like a reasonable idea to me. We already mandate CLang or
> GCC as only supported compilers, so there's no portability concerns.
> 
> 
> On the subject of non-standard C extensions, I was also wondering if
> QEMU should consider use of __attribute__((cleanup($func))) for doing
> automatically free'ing of memory when variables go out of scope. As
> well as reducing possibility of memory leaks, this simplifies code
> as it often allows for many "goto cleanup" jumps to be replaced with
> direct "return -1"s.  It isn't the most attractive syntax, but that
> downside doesn't outweigh the positives from my POV.
> 
> We recently started using it in libvirt for this reason.
> 
> Glib2 itself already uses this since 2.44 and exposes some macros for
> client code to use for this.
> 
> We bumped min glib to 2.40 a year go because Debian Jessie has 2.42
> and Ubuntu Trusty needs 2.40 which Peter used as a build merge test
> machine.
> 
> Our platform support policy will let us drop Jessie support when
> Buster is released fairly soon. If Peter no longer uses Trusty
> machines for buld, we'll soon be able to have a min glib that uses
> attribute(cleanup) as a standard dep.

That sounds good to me.  I've also seen __attribute__((cleanup($func)))
used more frequently in other projects and it makes the code both
simpler and safer.

Stefan