*** BLURB HERE *** Michal Prívozník (4): lib: Replace virBuildPath() with g_build_filename() virfile: Drop virBuildPathInternal() virSecretLoadAllConfigs: Use g_autofree for @path virSecretLoad: Simplify cleanup path docs/kbase/internals/command.rst | 2 +- src/conf/virsecretobj.c | 15 ++++++--------- src/libvirt_private.syms | 1 - src/util/virfcp.c | 2 +- src/util/virfile.c | 22 ---------------------- src/util/virfile.h | 5 ----- src/util/virhook.c | 4 ++-- src/util/virpci.c | 4 ++-- 8 files changed, 12 insertions(+), 43 deletions(-) -- 2.41.0
Our virBuildPath() constructs a path from given arguments. Exactly like g_build_filename(), except the latter is more generic as it uses backslashes on Windows. Therefore, replace the former with the latter. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/kbase/internals/command.rst | 2 +- src/util/virfcp.c | 2 +- src/util/virhook.c | 4 ++-- src/util/virpci.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/kbase/internals/command.rst b/docs/kbase/internals/command.rst index 738fb5930a..d958c9d16a 100644 --- a/docs/kbase/internals/command.rst +++ b/docs/kbase/internals/command.rst @@ -444,7 +444,7 @@ src/util/hooks.c g_autofree char *path = NULL; g_autoptr(virCommand) cmd = NULL; - virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr); + path = g_build_filename(LIBVIRT_HOOK_DIR, drvstr); cmd = virCommandNew(path); diff --git a/src/util/virfcp.c b/src/util/virfcp.c index bb62fa9025..052a6c99e1 100644 --- a/src/util/virfcp.c +++ b/src/util/virfcp.c @@ -38,7 +38,7 @@ virFCIsCapableRport(const char *rport) { g_autofree char *path = NULL; - virBuildPath(&path, SYSFS_FC_RPORT_PATH, rport); + path = g_build_filename(SYSFS_FC_RPORT_PATH, rport, NULL); return virFileExists(path); } diff --git a/src/util/virhook.c b/src/util/virhook.c index 50e178723f..d012bb1825 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -153,7 +153,7 @@ virHookCheck(int no, const char *driver) return -1; } - virBuildPath(&path, LIBVIRT_HOOK_DIR, driver); + path = g_build_filename(LIBVIRT_HOOK_DIR, driver, NULL); if (!virFileExists(path)) { VIR_DEBUG("No hook script %s", path); @@ -398,7 +398,7 @@ virHookCall(int driver, if (extra == NULL) extra = "-"; - virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr); + path = g_build_filename(LIBVIRT_HOOK_DIR, drvstr, NULL); script_ret = 1; diff --git a/src/util/virpci.c b/src/util/virpci.c index 08b82708b1..baacde4c14 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2396,7 +2396,7 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path, *pf = NULL; - virBuildPath(&device_link, vf_sysfs_path, "physfn"); + device_link = g_build_filename(vf_sysfs_path, "physfn", NULL); if ((*pf = virPCIGetDeviceAddressFromSysfsLink(device_link))) { VIR_DEBUG("PF for VF device '%s': " VIR_PCI_DEVICE_ADDRESS_FMT, @@ -2580,7 +2580,7 @@ virPCIGetNetName(const char *device_link_sysfs_path, return -1; } - virBuildPath(&pcidev_sysfs_net_path, device_link_sysfs_path, "net"); + pcidev_sysfs_net_path = g_build_filename(device_link_sysfs_path, "net", NULL); if (virDirOpenQuiet(&dir, pcidev_sysfs_net_path) < 0) { /* this *isn't* an error - caller needs to check for netname == NULL */ -- 2.41.0
On Mon, Oct 16, 2023 at 10:07:48 +0200, Michal Privoznik wrote:
Our virBuildPath() constructs a path from given arguments. Exactly like g_build_filename(), except the latter is more generic as it uses backslashes on Windows. Therefore, replace the former with the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/kbase/internals/command.rst | 2 +- src/util/virfcp.c | 2 +- src/util/virhook.c | 4 ++-- src/util/virpci.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/docs/kbase/internals/command.rst b/docs/kbase/internals/command.rst index 738fb5930a..d958c9d16a 100644 --- a/docs/kbase/internals/command.rst +++ b/docs/kbase/internals/command.rst @@ -444,7 +444,7 @@ src/util/hooks.c g_autofree char *path = NULL; g_autoptr(virCommand) cmd = NULL;
- virBuildPath(&path, LIBVIRT_HOOK_DIR, drvstr); + path = g_build_filename(LIBVIRT_HOOK_DIR, drvstr);
It's docs, but it's missing the NULL sentinel. Reviewed-by: Peter Krempa <pkrempa@redhat.com>
After previous cleanup the virBuildPathInternal() function is no longer used. Drop it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virfile.c | 22 ---------------------- src/util/virfile.h | 5 ----- 3 files changed, 28 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4e475d5b1a..553b01b8c0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2280,7 +2280,6 @@ virFDStreamSetInternalCloseCb; saferead; safewrite; safezero; -virBuildPathInternal; virCloseFrom; virCloseRange; virCloseRangeInit; diff --git a/src/util/virfile.c b/src/util/virfile.c index bd36a9a31a..54708652fb 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1490,28 +1490,6 @@ virFileFindMountPoint(const char *type G_GNUC_UNUSED) #endif /* defined WITH_MNTENT_H && defined WITH_GETMNTENT_R */ -void -virBuildPathInternal(char **path, ...) -{ - char *path_component = NULL; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - va_list ap; - - va_start(ap, path); - - path_component = va_arg(ap, char *); - virBufferAdd(&buf, path_component, -1); - - while ((path_component = va_arg(ap, char *)) != NULL) { - virBufferAddChar(&buf, '/'); - virBufferAdd(&buf, path_component, -1); - } - - va_end(ap); - - *path = virBufferContentAndReset(&buf); -} - /* Read no more than the specified maximum number of bytes. */ static char * saferead_lim(int fd, size_t max_len, size_t *length) diff --git a/src/util/virfile.h b/src/util/virfile.h index adc032ba33..286401e0f5 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -307,11 +307,6 @@ int virFileOpenTty(int *ttymaster, char *virFileFindMountPoint(const char *type); -/* NB: this should be combined with virFileBuildPath */ -#define virBuildPath(path, ...) \ - virBuildPathInternal(path, __VA_ARGS__, NULL) -void virBuildPathInternal(char **path, ...) G_GNUC_NULL_TERMINATED; - typedef struct _virHugeTLBFS virHugeTLBFS; struct _virHugeTLBFS { char *mnt_dir; /* Where the FS is mount to */ -- 2.41.0
On Mon, Oct 16, 2023 at 10:07:49 +0200, Michal Privoznik wrote:
After previous cleanup the virBuildPathInternal() function is no longer used. Drop it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virfile.c | 22 ---------------------- src/util/virfile.h | 5 ----- 3 files changed, 28 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When loading virSecret configs, the @path variable holds path to individual config files. In each iteration it is freed explicitly using VIR_FREE(). Switch it to g_autofree and remove those explicit calls. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/virsecretobj.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index e0393e6cec..2585b2c972 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -902,7 +902,7 @@ virSecretLoadAllConfigs(virSecretObjList *secrets, /* Ignore errors reported by readdir or other calls within the * loop (if any). It's better to keep the secrets we managed to find. */ while (virDirRead(dir, &de, NULL) > 0) { - char *path; + g_autofree char *path = NULL; virSecretObj *obj; if (!virStringHasSuffix(de->d_name, ".xml")) @@ -914,11 +914,9 @@ virSecretLoadAllConfigs(virSecretObjList *secrets, if (!(obj = virSecretLoad(secrets, de->d_name, path, configDir))) { VIR_ERROR(_("Error reading secret: %1$s"), virGetLastErrorMessage()); - VIR_FREE(path); continue; } - VIR_FREE(path); virSecretObjEndAPI(&obj); } -- 2.41.0
On Mon, Oct 16, 2023 at 10:07:50 +0200, Michal Privoznik wrote:
When loading virSecret configs, the @path variable holds path to individual config files. In each iteration it is freed explicitly using VIR_FREE(). Switch it to g_autofree and remove those explicit calls.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/virsecretobj.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When loading a secret value fails, the control jumps over to the 'cleanup' label where explicit call to virSecretDefFree() happens. This is unnecessary as the corresponding variable can be declared with g_autoptr() after which all error paths can just return NULL instantly. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/virsecretobj.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 2585b2c972..455798d414 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -865,25 +865,24 @@ virSecretLoad(virSecretObjList *secrets, const char *path, const char *configDir) { - virSecretDef *def = NULL; + g_autoptr(virSecretDef) def = NULL; virSecretObj *obj = NULL; if (!(def = virSecretDefParse(NULL, path, 0))) - goto cleanup; + return NULL; if (virSecretLoadValidateUUID(def, file) < 0) - goto cleanup; + return NULL; if (!(obj = virSecretObjListAdd(secrets, &def, configDir, NULL))) - goto cleanup; + return NULL; if (virSecretLoadValue(obj) < 0) { virSecretObjListRemove(secrets, obj); g_clear_pointer(&obj, virObjectUnref); + return NULL; } - cleanup: - virSecretDefFree(def); return obj; } -- 2.41.0
On Mon, Oct 16, 2023 at 10:07:51 +0200, Michal Privoznik wrote:
When loading a secret value fails, the control jumps over to the 'cleanup' label where explicit call to virSecretDefFree() happens. This is unnecessary as the corresponding variable can be declared with g_autoptr() after which all error paths can just return NULL instantly.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/virsecretobj.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
On a Monday in 2023, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (4): lib: Replace virBuildPath() with g_build_filename() virfile: Drop virBuildPathInternal() virSecretLoadAllConfigs: Use g_autofree for @path virSecretLoad: Simplify cleanup path
docs/kbase/internals/command.rst | 2 +- src/conf/virsecretobj.c | 15 ++++++--------- src/libvirt_private.syms | 1 - src/util/virfcp.c | 2 +- src/util/virfile.c | 22 ---------------------- src/util/virfile.h | 5 ----- src/util/virhook.c | 4 ++-- src/util/virpci.c | 4 ++-- 8 files changed, 12 insertions(+), 43 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Ján Tomko -
Michal Privoznik -
Peter Krempa