[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