[PATCH 0/9] Cleanup of qemu_oom_check() and qemu_memalign()

[Peter Maydell]

This series does some cleanup of the qemu_oom_check() and
qemu_memalign() functions; I started looking at the first of these and
found myself wanting to tidy some stuff relating to the second in the
process. The TLDR is that this series removes qemu_oom_check() (which
was mostly being misused), unifies the POSIX and Win32 versions of
qemu_memalign() and qemu_try_memalign(), and moves the prototypes out
of osdep.h.

qemu_oom_check() is a function which essentially says "if you pass me
a NULL pointer then print a message and abort()".  On POSIX systems
the message includes strerror(errno); on Windows it includes the
GetLastError() error value printed as an integer.  Other than in the
implementation of qemu_memalign(), we use this function only in
hw/usb/redirect.c, for three checks:
 * on a call to usbredirparser_create()
 * on a call to usberedirparser_serialize()
 * on a call to malloc()

The usbredir library API functions make no guarantees that they will
set errno on errors, let alone that they might set the
Windows-specific GetLastError string. So in 2/3 of the cases using
qemu_oom_check() is definitely wrong, and in the third it's dubious
given that Windows malloc() doesn't set GetLastError (more on that
later). So we start by switching these 3 calls to simple in-line error
checking code, which means we can make qemu_oom_check() not a public
global function.

So now we have a qemu_oom_check() which is used only in
qemu_memalign() and whose major difference between the Windows and
POSIX versions is that the former prints the GetLastError error number
value. This is actually a bug -- in commit dfbd0b873a85021 in 2020 we
changed the implementation of qemu_try_memalign() from using
VirtualAlloc() without noticing that this also should imply changing
what we print in the error case. So we can switch to a single
shared qemu_memalign() which does the error check directly itself
and always prints errno (and the requested size and alignment,
which seem more useful if anybody needs to diagnose the problem).

qemu_try_memalign() also is very similar between POSIX and Windows
versions (it used to be less so, but we have gradually ended up
with the two versions quite similar). The POSIX version already
has an ifdef ladder for different kinds of "allocate aligned
memory" host functions, so it seems neater to just have Windows'
_aligned_malloc() be another ladder in that ifdef rather than
a totally separate function.

Finally, I put the qemu_memalign() and related function prototypes
into a new memalign.h header, because that gets them out of osdep.h
and into somewhere that only the files that care about these functions
need to include.

thanks
--PMM

Peter Maydell (9):
  hw/usb/redirect.c: Stop using qemu_oom_check()
  util: Make qemu_oom_check() a static function
  util: Unify implementations of qemu_memalign()
  util/oslib-win32: Return NULL on qemu_try_memalign() with zero size
  meson.build: Don't misdetect posix_memalign() on Windows
  util: Share qemu_try_memalign() implementation between POSIX and
    Windows
  util: Use meson checks for valloc() and memalign() presence
  util: Put qemu_vfree() in memalign.c
  osdep: Move memalign-related functions to their own header

 meson.build                    |  7 ++-
 include/qemu-common.h          |  2 -
 include/qemu/memalign.h        | 61 +++++++++++++++++++++++
 include/qemu/osdep.h           | 18 -------
 block/blkverify.c              |  1 +
 block/block-copy.c             |  1 +
 block/commit.c                 |  1 +
 block/crypto.c                 |  1 +
 block/dmg.c                    |  1 +
 block/export/fuse.c            |  1 +
 block/file-posix.c             |  1 +
 block/io.c                     |  1 +
 block/mirror.c                 |  1 +
 block/nvme.c                   |  1 +
 block/parallels-ext.c          |  1 +
 block/parallels.c              |  1 +
 block/qcow.c                   |  1 +
 block/qcow2-cache.c            |  1 +
 block/qcow2-cluster.c          |  1 +
 block/qcow2-refcount.c         |  1 +
 block/qcow2-snapshot.c         |  1 +
 block/qcow2.c                  |  1 +
 block/qed-l2-cache.c           |  1 +
 block/qed-table.c              |  1 +
 block/qed.c                    |  1 +
 block/quorum.c                 |  1 +
 block/raw-format.c             |  1 +
 block/vdi.c                    |  1 +
 block/vhdx-log.c               |  1 +
 block/vhdx.c                   |  1 +
 block/vmdk.c                   |  1 +
 block/vpc.c                    |  1 +
 block/win32-aio.c              |  1 +
 hw/block/dataplane/xen-block.c |  1 +
 hw/block/fdc.c                 |  1 +
 hw/ide/core.c                  |  1 +
 hw/ppc/spapr.c                 |  1 +
 hw/ppc/spapr_softmmu.c         |  1 +
 hw/scsi/scsi-disk.c            |  1 +
 hw/tpm/tpm_ppi.c               |  2 +-
 hw/usb/redirect.c              | 17 +++++--
 nbd/server.c                   |  1 +
 net/l2tpv3.c                   |  2 +-
 plugins/loader.c               |  1 +
 qemu-img.c                     |  1 +
 qemu-io-cmds.c                 |  1 +
 qom/object.c                   |  1 +
 softmmu/physmem.c              |  1 +
 target/i386/hvf/hvf.c          |  1 +
 target/i386/kvm/kvm.c          |  1 +
 tcg/region.c                   |  1 +
 tests/bench/atomic_add-bench.c |  1 +
 tests/bench/qht-bench.c        |  1 +
 util/atomic64.c                |  1 +
 util/memalign.c                | 88 ++++++++++++++++++++++++++++++++++
 util/oslib-posix.c             | 46 ------------------
 util/oslib-win32.c             | 35 --------------
 util/qht.c                     |  1 +
 util/meson.build               |  1 +
 59 files changed, 220 insertions(+), 107 deletions(-)
 create mode 100644 include/qemu/memalign.h
 create mode 100644 util/memalign.c

-- 
2.25.1