Hey,
On 10/27/20 10:35 PM, Laine Stump wrote:
I don't even remember why I started looking at this. I think that
again I was considering changing some function, and making the DIR* in
that function autoclose was a prerequisite for a reasonably clean
addition to the function. I eventually gave up on the other plan
(probably because it was a bad idea), but decided that the DIR*'s
really did need to autoclose.
In the end, all uses of DIR* could be easily converted to use
g_autoptr.
I think you created this series in parallel with your "node_device: fix leak
of DIR*" patch, right?. Because you're not changing 'node_device_udev.c' anywhere in
this series, meaning that the code base you used still have the DIR leak in
udevGetVDPACharDev(). The code base didn't have the added VIR_DIR_CLOSE() calls there
to fix the leak, and I believe you forgot to take that into account here. The end result
is that the leak fix patch will break after patch 10 removes the VIR_DIR_CLOSE()
macro.
A quick fix is simply using "node_device: fix leak of DIR*" as the first
patch of this series, then you can handle the removal of VIR_DIR_CLOSE()
and g_autoptr() for that case in patch 8. There's no cleanup labels to
be added there, so it's a trivial removal of the two VIR_DIR_CLOSE()
calls and turning the DIR var to g_autoptr().
Assuming you go with that (and fix my whitespace nit in patch 12! :P ), feel
free to add my R-b in all patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
Thanks,
DHB
Laine Stump (12):
consistently use VIR_DIR_CLOSE() instead of virDirClose()
tools: reduce scope of a DIR* in virHostValidateIOMMU()
storage: remove extraneous call to VIR_DIR_CLOSE()
util: reduce scope of a DIR * in virCgroupV1SetOwner()
util: manually set dirp to NULL after closing in
virCapabilitiesInitCache()
util: change virDirClose to take a DIR* instead of DIR**.
util: declare g_autoptr cleanup function to auto-close DIR*
change DIR* int g_autoptr(DIR) where appropriate
conf: convert final DIR* to g_autoptr
util: remove unused VIR_DIR_CLOSE() macro
util: refactor function to simplify and remove label
remove unnecessary cleanup labels and unused return variables
src/bhyve/bhyve_capabilities.c | 3 +-
src/conf/capabilities.c | 5 +-
src/conf/virdomainobjlist.c | 3 +-
src/conf/virnetworkobj.c | 44 +++++---------
src/conf/virnwfilterbindingobjlist.c | 3 +-
src/conf/virnwfilterobj.c | 3 +-
src/conf/virsecretobj.c | 3 +-
src/conf/virstorageobj.c | 6 +-
src/openvz/openvz_conf.c | 3 +-
src/qemu/qemu_driver.c | 6 +-
src/qemu/qemu_interop_config.c | 14 ++---
src/security/security_selinux.c | 8 +--
src/storage/storage_backend_iscsi.c | 3 +-
src/storage/storage_util.c | 69 ++++++++-------------
src/util/vircgroup.c | 23 +++----
src/util/vircgroupv1.c | 5 +-
src/util/vircommand.c | 12 ++--
src/util/virdevmapper.c | 9 +--
src/util/virfile.c | 65 ++++++++------------
src/util/virfile.h | 4 +-
src/util/virhook.c | 8 +--
src/util/virhostcpu.c | 6 +-
src/util/virnetdev.c | 13 ++--
src/util/virnuma.c | 24 +++-----
src/util/virpci.c | 91 +++++++++++-----------------
src/util/virprocess.c | 3 +-
src/util/virresctrl.c | 30 ++++-----
src/util/virscsi.c | 32 ++++------
src/util/virscsihost.c | 3 +-
src/util/virusb.c | 3 +-
src/util/virutil.c | 26 +++-----
src/util/virvhba.c | 20 +++---
tests/testutilsqemu.c | 35 ++++-------
tests/virschematest.c | 3 +-
tools/virt-host-validate-common.c | 4 +-
35 files changed, 206 insertions(+), 386 deletions(-)
