[PATCH 00/12] use g_autoptr for all DIR*
Daniel Henrique Barboza
danielhb413 at gmail.com
Wed Oct 28 11:39:22 UTC 2020
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 at gmail.com>
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(-)
>
More information about the libvir-list
mailing list