[PATCH 01/30] util: convert pointers to use g_autofree

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virxml.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virxml.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index a3b819d85c..7df50e4b4d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -696,8 +696,8 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) unsigned int n, col; /* GCC warns if signed, because compared with sizeof() */ int domcode = VIR_FROM_XML; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char *contextstr = NULL; - char *pointerstr = NULL; + g_autofree char *contextstr = NULL; + g_autofree char *pointerstr = NULL; /* conditions for error printing */ @@ -763,9 +763,6 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) contextstr, pointerstr); } - - VIR_FREE(contextstr); - VIR_FREE(pointerstr); } /** -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virvhba.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virvhba.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/src/util/virvhba.c b/src/util/virvhba.c index a4e88024d1..a80145b8fd 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c @@ -49,7 +49,7 @@ bool virVHBAPathExists(const char *sysfs_prefix, int host) { - char *sysfs_path = NULL; + g_autofree char *sysfs_path = NULL; bool ret = false; sysfs_path = g_strdup_printf("%s/host%d", @@ -58,7 +58,6 @@ virVHBAPathExists(const char *sysfs_prefix, if (virFileExists(sysfs_path)) ret = true; - VIR_FREE(sysfs_path); return ret; } @@ -79,8 +78,8 @@ bool virVHBAIsVportCapable(const char *sysfs_prefix, int host) { - char *scsi_host_path = NULL; - char *fc_host_path = NULL; + g_autofree char *scsi_host_path = NULL; + g_autofree char *fc_host_path = NULL; bool ret = false; fc_host_path = g_strdup_printf("%s/host%d/%s", @@ -94,8 +93,6 @@ virVHBAIsVportCapable(const char *sysfs_prefix, if (virFileExists(fc_host_path) || virFileExists(scsi_host_path)) ret = true; - VIR_FREE(fc_host_path); - VIR_FREE(scsi_host_path); return ret; } @@ -115,9 +112,9 @@ virVHBAGetConfig(const char *sysfs_prefix, int host, const char *entry) { - char *sysfs_path = NULL; + g_autofree char *sysfs_path = NULL; char *p = NULL; - char *buf = NULL; + g_autofree char *buf = NULL; char *result = NULL; sysfs_path = g_strdup_printf("%s/host%d/%s", @@ -140,8 +137,6 @@ virVHBAGetConfig(const char *sysfs_prefix, result = g_strdup(p); cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); return result; } @@ -160,8 +155,8 @@ virVHBAFindVportHost(const char *sysfs_prefix) const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; g_autoptr(DIR) dir = NULL; struct dirent *entry = NULL; - char *max_vports = NULL; - char *vports = NULL; + g_autofree char *max_vports = NULL; + g_autofree char *vports = NULL; char *state = NULL; char *ret = NULL; @@ -220,8 +215,6 @@ virVHBAFindVportHost(const char *sysfs_prefix) } cleanup: - VIR_FREE(max_vports); - VIR_FREE(vports); return ret; } @@ -241,7 +234,8 @@ virVHBAManageVport(const int parent_host, int operation) { int ret = -1; - char *operation_path = NULL, *vport_name = NULL; + g_autofree char *operation_path = NULL; + g_autofree char *vport_name = NULL; const char *operation_file = NULL; switch (operation) { @@ -291,8 +285,6 @@ virVHBAManageVport(const int parent_host, vport_name, operation_path); cleanup: - VIR_FREE(vport_name); - VIR_FREE(operation_path); return ret; } @@ -315,8 +307,8 @@ vhbaReadCompareWWN(const char *prefix, const char *f_name, const char *wwn) { - char *path; - char *buf = NULL; + g_autofree char *path = NULL; + g_autofree char *buf = NULL; char *p; int ret = -1; @@ -343,8 +335,6 @@ vhbaReadCompareWWN(const char *prefix, ret = 1; cleanup: - VIR_FREE(path); - VIR_FREE(buf); return ret; } @@ -407,7 +397,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *vport_create_path = NULL; + g_autofree char *vport_create_path = NULL; char *ret = NULL; if (virDirOpen(&dir, prefix) < 0) @@ -438,7 +428,6 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, } cleanup: - VIR_FREE(vport_create_path); return ret; } -- 2.29.0

util: vhba: convert pointers to use g_autofree On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virvhba.c
No need to mention the file here.
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virvhba.c | 35 ++++++++++++----------------------- 1 file changed, 12 insertions(+), 23 deletions(-)
diff --git a/src/util/virvhba.c b/src/util/virvhba.c index a4e88024d1..a80145b8fd 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c
@@ -160,8 +155,8 @@ virVHBAFindVportHost(const char *sysfs_prefix) const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; g_autoptr(DIR) dir = NULL; struct dirent *entry = NULL; - char *max_vports = NULL; - char *vports = NULL; + g_autofree char *max_vports = NULL; + g_autofree char *vports = NULL; char *state = NULL; char *ret = NULL;
@@ -220,8 +215,6 @@ virVHBAFindVportHost(const char *sysfs_prefix) }
cleanup: - VIR_FREE(max_vports); - VIR_FREE(vports);
These two are also freed within the while loop above. We try to avoid mixing autofree with calls to VIR_FREE/g_free, to make things simple. Here, you can simply move the declaration of the two variables at the top of the loop, since they're freed onece per loop iteration. Same for the 'state' variable.
return ret; }
@@ -241,7 +234,8 @@ virVHBAManageVport(const int parent_host, int operation) { int ret = -1; - char *operation_path = NULL, *vport_name = NULL; + g_autofree char *operation_path = NULL; + g_autofree char *vport_name = NULL; const char *operation_file = NULL;
switch (operation) { @@ -291,8 +285,6 @@ virVHBAManageVport(const int parent_host, vport_name, operation_path);
cleanup: - VIR_FREE(vport_name); - VIR_FREE(operation_path);
operation_path is also freed in the middle of the function. But I cannot think of an elegant solution to that one.
return ret; }
@@ -315,8 +307,8 @@ vhbaReadCompareWWN(const char *prefix, const char *f_name, const char *wwn) { - char *path; - char *buf = NULL; + g_autofree char *path = NULL; + g_autofree char *buf = NULL; char *p; int ret = -1;
@@ -343,8 +335,6 @@ vhbaReadCompareWWN(const char *prefix, ret = 1;
cleanup: - VIR_FREE(path); - VIR_FREE(buf);
return ret; } @@ -407,7 +397,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *vport_create_path = NULL; + g_autofree char *vport_create_path = NULL;
This one should be also moved into the while loop. Jano
char *ret = NULL;
if (virDirOpen(&dir, prefix) < 0) @@ -438,7 +428,6 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, }
cleanup: - VIR_FREE(vport_create_path); return ret; }
-- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virutil.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virutil.c | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index a0cd0f1bcd..d4b864d5cb 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -598,7 +598,7 @@ char *virGetUserRuntimeDirectory(void) static int virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bool quiet) { - char *strbuf; + g_autofree char *strbuf = NULL; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); @@ -668,13 +668,12 @@ virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bo if (shell) VIR_FREE(*shell); } - VIR_FREE(strbuf); return ret; } static char *virGetGroupEnt(gid_t gid) { - char *strbuf; + g_autofree char *strbuf = NULL; char *ret; struct group grbuf; struct group *gr = NULL; @@ -717,7 +716,6 @@ static char *virGetGroupEnt(gid_t gid) } ret = g_strdup(gr->gr_name); - VIR_FREE(strbuf); return ret; } @@ -759,7 +757,7 @@ char *virGetGroupName(gid_t gid) static int virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) { - char *strbuf = NULL; + g_autofree char *strbuf = NULL; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); @@ -796,8 +794,6 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) ret = 0; cleanup: - VIR_FREE(strbuf); - return ret; } @@ -840,7 +836,7 @@ virGetUserID(const char *user, uid_t *uid) static int virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) { - char *strbuf = NULL; + g_autofree char *strbuf = NULL; struct group grbuf; struct group *gr = NULL; long val = sysconf(_SC_GETGR_R_SIZE_MAX); @@ -877,8 +873,6 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) ret = 0; cleanup: - VIR_FREE(strbuf); - return ret; } @@ -949,7 +943,7 @@ int virGetGroupList(uid_t uid, gid_t gid, gid_t **list) { int ret = 0; - char *user = NULL; + g_autofree char *user = NULL; gid_t primary; *list = NULL; @@ -999,7 +993,6 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list) } cleanup: - VIR_FREE(user); return ret; } @@ -1405,8 +1398,8 @@ virSetDeviceUnprivSGIO(const char *path, const char *sysfs_dir, int unpriv_sgio) { - char *sysfs_path = NULL; - char *val = NULL; + g_autofree char *sysfs_path = NULL; + g_autofree char *val = NULL; int ret = -1; int rc; @@ -1428,8 +1421,6 @@ virSetDeviceUnprivSGIO(const char *path, ret = 0; cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(val); return ret; } @@ -1438,8 +1429,8 @@ virGetDeviceUnprivSGIO(const char *path, const char *sysfs_dir, int *unpriv_sgio) { - char *sysfs_path = NULL; - char *buf = NULL; + g_autofree char *sysfs_path = NULL; + g_autofree char *buf = NULL; char *tmp = NULL; int ret = -1; @@ -1466,8 +1457,6 @@ virGetDeviceUnprivSGIO(const char *path, ret = 0; cleanup: - VIR_FREE(sysfs_path); - VIR_FREE(buf); return ret; } @@ -1488,7 +1477,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) int rc = -1; uid_t theuid; gid_t thegid; - char *tmp_label = NULL; + g_autofree char *tmp_label = NULL; char *sep = NULL; char *owner = NULL; char *group = NULL; @@ -1522,8 +1511,6 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) rc = 0; cleanup: - VIR_FREE(tmp_label); - return rc; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virutil.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virutil.c | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c index a0cd0f1bcd..d4b864d5cb 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -598,7 +598,7 @@ char *virGetUserRuntimeDirectory(void) static int virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bool quiet) { - char *strbuf; + g_autofree char *strbuf = NULL; struct passwd pwbuf; struct passwd *pw = NULL; long val = sysconf(_SC_GETPW_R_SIZE_MAX); @@ -668,13 +668,12 @@ virGetUserEnt(uid_t uid, char **name, gid_t *group, char **dir, char **shell, bo if (shell) VIR_FREE(*shell); } - VIR_FREE(strbuf); return ret; }
static char *virGetGroupEnt(gid_t gid) { - char *strbuf; + g_autofree char *strbuf = NULL;
There are multiple cases of VIR_FREE(strbuf) left in this function.
char *ret; struct group grbuf; struct group *gr = NULL;
Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/viruri.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/viruri.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util/viruri.c b/src/util/viruri.c index 11753a0aef..704e5b2132 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -209,7 +209,7 @@ char * virURIFormat(virURIPtr uri) { xmlURI xmluri; - char *tmpserver = NULL; + g_autofree char *tmpserver = NULL; char *ret; memset(&xmluri, 0, sizeof(xmluri)); @@ -245,8 +245,6 @@ virURIFormat(virURIPtr uri) } cleanup: - VIR_FREE(tmpserver); - return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/viruri.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/viruri.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virsysinfo.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virsysinfo.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 217f842a37..36317c97eb 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -393,7 +393,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) const char *cur; char *eol, *tmp_base; virSysinfoProcessorDefPtr processor; - char *processor_type = NULL; + g_autofree char *processor_type = NULL; if (!(tmp_base = strstr(base, "model name")) && !(tmp_base = strstr(base, "Processor"))) @@ -424,12 +424,10 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) base = cur; } - VIR_FREE(processor_type); return 0; error: - VIR_FREE(processor_type); - return -1; + return -1; } /* virSysinfoRead for ARMv7 @@ -532,9 +530,9 @@ static int virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) { const char *tmp_base; - char *manufacturer = NULL; - char *procline = NULL; - char *ncpu = NULL; + g_autofree char *manufacturer = NULL; + g_autofree char *procline = NULL; + g_autofree char *ncpu = NULL; int result = -1; virSysinfoProcessorDefPtr processor; @@ -593,9 +591,6 @@ virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) result = 0; error: - VIR_FREE(manufacturer); - VIR_FREE(procline); - VIR_FREE(ncpu); return result; } -- 2.29.0

util: sysinfo: in the commit summary On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virsysinfo.c
No need to mention the file.
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virsysinfo.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 217f842a37..36317c97eb 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -393,7 +393,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) const char *cur; char *eol, *tmp_base; virSysinfoProcessorDefPtr processor; - char *processor_type = NULL; + g_autofree char *processor_type = NULL;
if (!(tmp_base = strstr(base, "model name")) && !(tmp_base = strstr(base, "Processor"))) @@ -424,12 +424,10 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) base = cur; }
- VIR_FREE(processor_type); return 0;
error: - VIR_FREE(processor_type);
- return -1; + return -1;
Unrelated whitespace change.
}
/* virSysinfoRead for ARMv7 @@ -532,9 +530,9 @@ static int virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) { const char *tmp_base; - char *manufacturer = NULL; - char *procline = NULL; - char *ncpu = NULL; + g_autofree char *manufacturer = NULL;
+ g_autofree char *procline = NULL; + g_autofree char *ncpu = NULL;
These two belong in their respective loops below. Jano
int result = -1; virSysinfoProcessorDefPtr processor;
@@ -593,9 +591,6 @@ virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret) result = 0;
error: - VIR_FREE(manufacturer); - VIR_FREE(procline); - VIR_FREE(ncpu); return result; }
-- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virstoragefilebackend.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virstoragefilebackend.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util/virstoragefilebackend.c b/src/util/virstoragefilebackend.c index 2779b5c307..55c62b0212 100644 --- a/src/util/virstoragefilebackend.c +++ b/src/util/virstoragefilebackend.c @@ -51,7 +51,7 @@ virStorageFileLoadBackendModule(const char *name, const char *regfunc, bool forceload) { - char *modfile = NULL; + g_autofree char *modfile = NULL; int ret; if (!(modfile = virFileFindResourceFull(name, @@ -64,8 +64,6 @@ virStorageFileLoadBackendModule(const char *name, ret = virModuleLoad(modfile, regfunc, forceload); - VIR_FREE(modfile); - return ret; } #endif /* WITH_STORAGE_DIR || WITH_STORAGE_FS || WITH_STORAGE_GLUSTER */ -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virstoragefilebackend.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virstoragefilebackend.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virstorageencryption.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virstorageencryption.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index 399c6926bd..ed29b4cfa5 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -142,7 +142,7 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, { VIR_XPATH_NODE_AUTORESTORE(ctxt) virStorageEncryptionSecretPtr ret; - char *type_str = NULL; + g_autofree char *type_str = NULL; ret = g_new0(virStorageEncryptionSecret, 1); @@ -164,12 +164,9 @@ virStorageEncryptionSecretParse(xmlXPathContextPtr ctxt, if (virSecretLookupParseSecret(node, &ret->seclookupdef) < 0) goto cleanup; - VIR_FREE(type_str); - return ret; cleanup: - VIR_FREE(type_str); virStorageEncryptionSecretFree(ret); return NULL; } @@ -180,7 +177,7 @@ virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, virStorageEncryptionInfoDefPtr info) { int ret = -1; - char *size_str = NULL; + g_autofree char *size_str = NULL; if (!(info->cipher_name = virXMLPropString(info_node, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -208,7 +205,6 @@ virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, ret = 0; cleanup: - VIR_FREE(size_str); return ret; } @@ -237,7 +233,7 @@ virStorageEncryptionParseNode(xmlNodePtr node, xmlNodePtr *nodes = NULL; virStorageEncryptionPtr encdef = NULL; virStorageEncryptionPtr ret = NULL; - char *format_str = NULL; + g_autofree char *format_str = NULL; int n; size_t i; @@ -297,7 +293,6 @@ virStorageEncryptionParseNode(xmlNodePtr node, ret = g_steal_pointer(&encdef); cleanup: - VIR_FREE(format_str); VIR_FREE(nodes); virStorageEncryptionFree(encdef); -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virstorageencryption.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virstorageencryption.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virsecret.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virsecret.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 54d6bbcb7c..78a3b41bc5 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -65,8 +65,8 @@ int virSecretLookupParseSecret(xmlNodePtr secretnode, virSecretLookupTypeDefPtr def) { - char *uuid; - char *usage; + g_autofree char *uuid = NULL; + g_autofree char *usage = NULL; int ret = -1; uuid = virXMLPropString(secretnode, "uuid"); @@ -98,8 +98,6 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, ret = 0; cleanup: - VIR_FREE(uuid); - VIR_FREE(usage); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virsecret.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virsecret.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virscsihost.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virscsihost.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c index 969cdd9f79..2ce33e4cfb 100644 --- a/src/util/virscsihost.c +++ b/src/util/virscsihost.c @@ -95,12 +95,12 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *host_link = NULL; - char *host_path = NULL; + g_autofree char *host_link = NULL; + g_autofree char *host_path = NULL; char *p = NULL; char *ret = NULL; - char *buf = NULL; - char *unique_path = NULL; + g_autofree char *buf = NULL; + g_autofree char *unique_path = NULL; unsigned int read_unique_id; if (virDirOpen(&dir, prefix) < 0) @@ -151,10 +151,6 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, } cleanup: - VIR_FREE(unique_path); - VIR_FREE(host_link); - VIR_FREE(host_path); - VIR_FREE(buf); return ret; } @@ -226,7 +222,7 @@ virSCSIHostGetNameByParentaddr(unsigned int domain, unsigned int unique_id) { char *name = NULL; - char *parentaddr = NULL; + g_autofree char *parentaddr = NULL; parentaddr = g_strdup_printf("%04x:%02x:%02x.%01x", domain, bus, slot, function); @@ -239,7 +235,6 @@ virSCSIHostGetNameByParentaddr(unsigned int domain, } cleanup: - VIR_FREE(parentaddr); return name; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virscsihost.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virscsihost.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c index 969cdd9f79..2ce33e4cfb 100644 --- a/src/util/virscsihost.c +++ b/src/util/virscsihost.c @@ -95,12 +95,12 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH; struct dirent *entry = NULL; g_autoptr(DIR) dir = NULL; - char *host_link = NULL; - char *host_path = NULL; + g_autofree char *host_link = NULL; + g_autofree char *host_path = NULL; char *p = NULL; char *ret = NULL; - char *buf = NULL; - char *unique_path = NULL; + g_autofree char *buf = NULL; + g_autofree char *unique_path = NULL; unsigned int read_unique_id;
if (virDirOpen(&dir, prefix) < 0) @@ -151,10 +151,6 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, }
cleanup: - VIR_FREE(unique_path); - VIR_FREE(host_link); - VIR_FREE(host_path); - VIR_FREE(buf);
All of these are also freed in the loop. Jano
return ret; }

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virrotatingfile.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virrotatingfile.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c index 9f1ef17c3e..6d8076e7c4 100644 --- a/src/util/virrotatingfile.c +++ b/src/util/virrotatingfile.c @@ -362,8 +362,8 @@ static int virRotatingFileWriterRollover(virRotatingFileWriterPtr file) { size_t i; - char *nextpath = NULL; - char *thispath = NULL; + g_autofree char *nextpath = NULL; + g_autofree char *thispath = NULL; int ret = -1; VIR_DEBUG("Rollover %s", file->basepath); @@ -403,8 +403,6 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) ret = 0; cleanup: - VIR_FREE(nextpath); - VIR_FREE(thispath); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virrotatingfile.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virrotatingfile.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c index 9f1ef17c3e..6d8076e7c4 100644 --- a/src/util/virrotatingfile.c +++ b/src/util/virrotatingfile.c @@ -362,8 +362,8 @@ static int virRotatingFileWriterRollover(virRotatingFileWriterPtr file) { size_t i; - char *nextpath = NULL; - char *thispath = NULL; + g_autofree char *nextpath = NULL; + g_autofree char *thispath = NULL; int ret = -1;
VIR_DEBUG("Rollover %s", file->basepath); @@ -403,8 +403,6 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file)
ret = 0; cleanup: - VIR_FREE(nextpath);
nextpath is also freed in the loop, but here we need to preserve its value across iterations. I'd rather leave it uncoverted. Jano
- VIR_FREE(thispath); return ret; }
-- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virresctrl.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virresctrl.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index d3087b98c1..1c2d175295 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -709,7 +709,7 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl) { int ret = -1; int rv = -1; - char *featurestr = NULL; + g_autofree char *featurestr = NULL; char **features = NULL; size_t nfeatures = 0; virResctrlInfoMongrpPtr info_monitor = NULL; @@ -771,7 +771,6 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl) ret = 0; cleanup: - VIR_FREE(featurestr); g_strfreev(features); VIR_FREE(info_monitor); return ret; @@ -1736,7 +1735,7 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, const char *groupname, virResctrlAllocPtr *alloc) { - char *schemata = NULL; + g_autofree char *schemata = NULL; int rv = virFileReadValueString(&schemata, SYSFS_RESCTRL_PATH "/%s/schemata", groupname); @@ -1753,11 +1752,9 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0) goto error; - VIR_FREE(schemata); return 0; error: - VIR_FREE(schemata); virObjectUnref(*alloc); *alloc = NULL; return -1; @@ -2354,8 +2351,8 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, const char *machinename) { - char *schemata_path = NULL; - char *alloc_str = NULL; + g_autofree char *schemata_path = NULL; + g_autofree char *alloc_str = NULL; int ret = -1; int lockfd = -1; @@ -2403,8 +2400,6 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, ret = 0; cleanup: virResctrlUnlock(lockfd); - VIR_FREE(alloc_str); - VIR_FREE(schemata_path); return ret; } @@ -2413,8 +2408,8 @@ static int virResctrlAddPID(const char *path, pid_t pid) { - char *tasks = NULL; - char *pidstr = NULL; + g_autofree char *tasks = NULL; + g_autofree char *pidstr = NULL; int ret = 0; if (!path) { @@ -2436,8 +2431,6 @@ virResctrlAddPID(const char *path, ret = 0; cleanup: - VIR_FREE(tasks); - VIR_FREE(pidstr); return ret; } @@ -2657,8 +2650,8 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, size_t i = 0; unsigned long long val = 0; g_autoptr(DIR) dirp = NULL; - char *datapath = NULL; - char *filepath = NULL; + g_autofree char *datapath = NULL; + g_autofree char *filepath = NULL; struct dirent *ent = NULL; virResctrlMonitorStatsPtr stat = NULL; @@ -2737,8 +2730,6 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, ret = 0; cleanup: - VIR_FREE(datapath); - VIR_FREE(filepath); virResctrlMonitorStatsFree(stat); return ret; } -- 2.29.0

util: resctl: convert pointers to use g_autofree On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virresctrl.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virresctrl.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index d3087b98c1..1c2d175295 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -709,7 +709,7 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl) { int ret = -1; int rv = -1; - char *featurestr = NULL; + g_autofree char *featurestr = NULL; char **features = NULL; size_t nfeatures = 0; virResctrlInfoMongrpPtr info_monitor = NULL; @@ -771,7 +771,6 @@ virResctrlGetMonitorInfo(virResctrlInfoPtr resctrl)
ret = 0; cleanup: - VIR_FREE(featurestr); g_strfreev(features); VIR_FREE(info_monitor); return ret; @@ -1736,7 +1735,7 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, const char *groupname, virResctrlAllocPtr *alloc) { - char *schemata = NULL; + g_autofree char *schemata = NULL; int rv = virFileReadValueString(&schemata, SYSFS_RESCTRL_PATH "/%s/schemata", groupname); @@ -1753,11 +1752,9 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl, if (virResctrlAllocParse(resctrl, *alloc, schemata) < 0) goto error;
- VIR_FREE(schemata); return 0;
error: - VIR_FREE(schemata); virObjectUnref(*alloc); *alloc = NULL; return -1; @@ -2354,8 +2351,8 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, virResctrlAllocPtr alloc, const char *machinename) { - char *schemata_path = NULL; - char *alloc_str = NULL; + g_autofree char *schemata_path = NULL; + g_autofree char *alloc_str = NULL; int ret = -1; int lockfd = -1;
@@ -2403,8 +2400,6 @@ virResctrlAllocCreate(virResctrlInfoPtr resctrl, ret = 0; cleanup: virResctrlUnlock(lockfd); - VIR_FREE(alloc_str); - VIR_FREE(schemata_path); return ret; }
@@ -2413,8 +2408,8 @@ static int virResctrlAddPID(const char *path, pid_t pid) { - char *tasks = NULL; - char *pidstr = NULL; + g_autofree char *tasks = NULL; + g_autofree char *pidstr = NULL; int ret = 0;
if (!path) { @@ -2436,8 +2431,6 @@ virResctrlAddPID(const char *path,
ret = 0; cleanup: - VIR_FREE(tasks); - VIR_FREE(pidstr); return ret; }
@@ -2657,8 +2650,8 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, size_t i = 0; unsigned long long val = 0; g_autoptr(DIR) dirp = NULL; - char *datapath = NULL; - char *filepath = NULL; + g_autofree char *datapath = NULL; + g_autofree char *filepath = NULL;
filepath should be moved into the loop. Jano
struct dirent *ent = NULL; virResctrlMonitorStatsPtr stat = NULL;
@@ -2737,8 +2730,6 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
ret = 0; cleanup: - VIR_FREE(datapath); - VIR_FREE(filepath); virResctrlMonitorStatsFree(stat); return ret; } -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virnetdevbandwidth.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virnetdevbandwidth.c | 44 ++++++++++++----------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index c8eb5cfc79..364b39e3c1 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -102,7 +102,7 @@ virNetDevBandwidthManipulateFilter(const char *ifname, bool create_new) { int ret = -1; - char *filter_id = NULL; + g_autofree char *filter_id = NULL; virCommandPtr cmd = NULL; unsigned char ifmac[VIR_MAC_BUFLEN]; char *mac[2] = {NULL, NULL}; @@ -157,7 +157,6 @@ virNetDevBandwidthManipulateFilter(const char *ifname, cleanup: VIR_FREE(mac[1]); VIR_FREE(mac[0]); - VIR_FREE(filter_id); virCommandFree(cmd); return ret; } @@ -195,9 +194,9 @@ virNetDevBandwidthSet(const char *ifname, int ret = -1; virNetDevBandwidthRatePtr rx = NULL, tx = NULL; /* From domain POV */ virCommandPtr cmd = NULL; - char *average = NULL; - char *peak = NULL; - char *burst = NULL; + g_autofree char *average = NULL; + g_autofree char *peak = NULL; + g_autofree char *burst = NULL; if (!bandwidth) { /* nothing to be enabled */ @@ -385,9 +384,6 @@ virNetDevBandwidthSet(const char *ifname, cleanup: virCommandFree(cmd); - VIR_FREE(average); - VIR_FREE(peak); - VIR_FREE(burst); return ret; } @@ -533,10 +529,10 @@ virNetDevBandwidthPlug(const char *brname, { int ret = -1; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *qdisc_id = NULL; - char *floor = NULL; - char *ceil = NULL; + g_autofree char *class_id = NULL; + g_autofree char *qdisc_id = NULL; + g_autofree char *floor = NULL; + g_autofree char *ceil = NULL; char ifmacStr[VIR_MAC_STRING_BUFLEN]; if (id <= 2) { @@ -586,10 +582,6 @@ virNetDevBandwidthPlug(const char *brname, ret = 0; cleanup: - VIR_FREE(ceil); - VIR_FREE(floor); - VIR_FREE(qdisc_id); - VIR_FREE(class_id); virCommandFree(cmd); return ret; } @@ -610,8 +602,8 @@ virNetDevBandwidthUnplug(const char *brname, int ret = -1; int cmd_ret = 0; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *qdisc_id = NULL; + g_autofree char *class_id = NULL; + g_autofree char *qdisc_id = NULL; if (id <= 2) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid class ID %d"), id); @@ -645,8 +637,6 @@ virNetDevBandwidthUnplug(const char *brname, ret = 0; cleanup: - VIR_FREE(qdisc_id); - VIR_FREE(class_id); virCommandFree(cmd); return ret; } @@ -673,9 +663,9 @@ virNetDevBandwidthUpdateRate(const char *ifname, { int ret = -1; virCommandPtr cmd = NULL; - char *class_id = NULL; - char *rate = NULL; - char *ceil = NULL; + g_autofree char *class_id = NULL; + g_autofree char *rate = NULL; + g_autofree char *ceil = NULL; class_id = g_strdup_printf("1:%x", id); rate = g_strdup_printf("%llukbps", new_rate); @@ -696,9 +686,6 @@ virNetDevBandwidthUpdateRate(const char *ifname, cleanup: virCommandFree(cmd); - VIR_FREE(class_id); - VIR_FREE(rate); - VIR_FREE(ceil); return ret; } @@ -725,7 +712,7 @@ virNetDevBandwidthUpdateFilter(const char *ifname, unsigned int id) { int ret = -1; - char *class_id = NULL; + g_autofree char *class_id = NULL; class_id = g_strdup_printf("1:%x", id); @@ -733,8 +720,7 @@ virNetDevBandwidthUpdateFilter(const char *ifname, class_id, true, true) < 0) goto cleanup; - ret = 0; + return 0; cleanup: - VIR_FREE(class_id); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virnetdevbandwidth.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virnetdevbandwidth.c | 44 ++++++++++++----------------------- 1 file changed, 15 insertions(+), 29 deletions(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index c8eb5cfc79..364b39e3c1 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -195,9 +194,9 @@ virNetDevBandwidthSet(const char *ifname, int ret = -1; virNetDevBandwidthRatePtr rx = NULL, tx = NULL; /* From domain POV */ virCommandPtr cmd = NULL; - char *average = NULL; - char *peak = NULL; - char *burst = NULL; + g_autofree char *average = NULL; + g_autofree char *peak = NULL; + g_autofree char *burst = NULL;
These are freed at multiple points in the function, but they can be declared at the start of each 'if' block they're used in.
if (!bandwidth) { /* nothing to be enabled */ @@ -385,9 +384,6 @@ virNetDevBandwidthSet(const char *ifname,
cleanup: virCommandFree(cmd); - VIR_FREE(average); - VIR_FREE(peak); - VIR_FREE(burst); return ret; }
[...]
@@ -733,8 +720,7 @@ virNetDevBandwidthUpdateFilter(const char *ifname, class_id, true, true) < 0) goto cleanup;
- ret = 0; + return 0;
Unrelated change. Jano
cleanup: - VIR_FREE(class_id); return ret; } -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virmacmap.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virmacmap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c index f9047d0fb1..e68742de10 100644 --- a/src/util/virmacmap.c +++ b/src/util/virmacmap.c @@ -129,7 +129,7 @@ static int virMacMapLoadFile(virMacMapPtr mgr, const char *file) { - char *map_str = NULL; + g_autofree char *map_str = NULL; virJSONValuePtr map = NULL; int map_str_len = 0; size_t i; @@ -189,7 +189,6 @@ virMacMapLoadFile(virMacMapPtr mgr, ret = 0; cleanup: - VIR_FREE(map_str); virJSONValueFree(map); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virmacmap.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virmacmap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virlog.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virlog.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 6b7a4512e9..e12fd58831 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -505,8 +505,8 @@ virLogVMessage(virLogSourcePtr source, va_list vargs) { static bool logInitMessageStderr = true; - char *str = NULL; - char *msg = NULL; + g_autofree char *str = NULL; + g_autofree char *msg = NULL; char timestamp[VIR_TIME_STRING_BUFLEN]; size_t i; int saved_errno = errno; @@ -601,10 +601,7 @@ virLogVMessage(virLogSourcePtr source, str, msg, (void *) STDERR_FILENO); } virLogUnlock(); - cleanup: - VIR_FREE(str); - VIR_FREE(msg); errno = saved_errno; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virlog.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virlog.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c index 6b7a4512e9..e12fd58831 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -505,8 +505,8 @@ virLogVMessage(virLogSourcePtr source, va_list vargs) { static bool logInitMessageStderr = true; - char *str = NULL; - char *msg = NULL; + g_autofree char *str = NULL; + g_autofree char *msg = NULL; char timestamp[VIR_TIME_STRING_BUFLEN]; size_t i; int saved_errno = errno; @@ -601,10 +601,7 @@ virLogVMessage(virLogSourcePtr source, str, msg, (void *) STDERR_FILENO); } virLogUnlock(); -
Unrelated whitespace change.
cleanup: - VIR_FREE(str); - VIR_FREE(msg); errno = saved_errno; }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virlockspace.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virlockspace.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index b90e13f506..71d5dfb83e 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -515,7 +515,7 @@ int virLockSpaceCreateResource(virLockSpacePtr lockspace, const char *resname) { int ret = -1; - char *respath = NULL; + g_autofree char *respath = NULL; VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname); @@ -538,7 +538,6 @@ int virLockSpaceCreateResource(virLockSpacePtr lockspace, cleanup: virMutexUnlock(&lockspace->lock); - VIR_FREE(respath); return ret; } @@ -547,7 +546,7 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace, const char *resname) { int ret = -1; - char *respath = NULL; + g_autofree char *respath = NULL; VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname); @@ -575,7 +574,6 @@ int virLockSpaceDeleteResource(virLockSpacePtr lockspace, cleanup: virMutexUnlock(&lockspace->lock); - VIR_FREE(respath); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virlockspace.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virlockspace.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virhostcpu.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virhostcpu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index c531d65f86..4f6c3390ce 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -87,7 +87,7 @@ virHostCPUGetStatsFreeBSD(int cpuNum, int *nparams) { const char *sysctl_name; - long *cpu_times; + g_autofree long *cpu_times = NULL; struct clockinfo clkinfo; size_t i, j, cpu_times_size, clkinfo_size; int cpu_times_num, offset, hz, stathz, ret = -1; @@ -172,8 +172,6 @@ virHostCPUGetStatsFreeBSD(int cpuNum, ret = 0; cleanup: - VIR_FREE(cpu_times); - return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virhostcpu.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virhostcpu.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virfile.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virfile.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index f57272ca2f..38207f1948 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3328,9 +3328,9 @@ virFileIsSharedFixFUSE(const char *path, FILE *f = NULL; struct mntent mb; char mntbuf[1024]; - char *mntDir = NULL; - char *mntType = NULL; - char *canonPath = NULL; + g_autofree char *mntDir = NULL; + g_autofree char *mntType = NULL; + g_autofree char *canonPath = NULL; size_t maxMatching = 0; int ret = -1; @@ -3381,9 +3381,6 @@ virFileIsSharedFixFUSE(const char *path, ret = 0; cleanup: - VIR_FREE(canonPath); - VIR_FREE(mntType); - VIR_FREE(mntDir); endmntent(f); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virfile.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virfile.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c index f57272ca2f..38207f1948 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -3328,9 +3328,9 @@ virFileIsSharedFixFUSE(const char *path, FILE *f = NULL; struct mntent mb; char mntbuf[1024]; - char *mntDir = NULL; - char *mntType = NULL;
These two are freed inside the loop, I'd rather not mix that with g_autofree. Jano
- char *canonPath = NULL; + g_autofree char *mntDir = NULL; + g_autofree char *mntType = NULL; + g_autofree char *canonPath = NULL; size_t maxMatching = 0; int ret = -1;
@@ -3381,9 +3381,6 @@ virFileIsSharedFixFUSE(const char *path,
ret = 0; cleanup: - VIR_FREE(canonPath); - VIR_FREE(mntType); - VIR_FREE(mntDir); endmntent(f); return ret; } -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virdnsmasq.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virdnsmasq.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 9030f9218a..5f477c976d 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -164,7 +164,7 @@ addnhostsWrite(const char *path, dnsmasqAddnHost *hosts, unsigned int nhosts) { - char *tmp; + g_autofree char *tmp = NULL; FILE *f; bool istmp = true; size_t i, j; @@ -230,8 +230,6 @@ addnhostsWrite(const char *path, } cleanup: - VIR_FREE(tmp); - return rc; } @@ -364,7 +362,7 @@ hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, unsigned int nhosts) { - char *tmp; + g_autofree char *tmp = NULL; FILE *f; bool istmp = true; size_t i; @@ -408,8 +406,6 @@ hostsfileWrite(const char *path, } cleanup: - VIR_FREE(tmp); - return rc; } @@ -686,7 +682,7 @@ static int dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path) { int ret = -1; - char *buf = NULL; + g_autofree char *buf = NULL; if (virFileReadAll(path, 1024 * 1024, &buf) < 0) goto cleanup; @@ -694,7 +690,6 @@ dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path) ret = dnsmasqCapsSetFromBuffer(caps, buf); cleanup: - VIR_FREE(buf); return ret; } @@ -704,7 +699,9 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force) int ret = -1; struct stat sb; virCommandPtr cmd = NULL; - char *help = NULL, *version = NULL, *complete = NULL; + g_autofree char *help = NULL; + g_autofree char *version = NULL; + g_autofree char *complete = NULL; if (!caps || caps->noRefresh) return 0; @@ -749,9 +746,6 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force) cleanup: virCommandFree(cmd); - VIR_FREE(help); - VIR_FREE(version); - VIR_FREE(complete); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virdnsmasq.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virdnsmasq.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/vircgroupv1.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/vircgroupv1.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c index 731e9d61d4..984cd50409 100644 --- a/src/util/vircgroupv1.c +++ b/src/util/vircgroupv1.c @@ -1549,7 +1549,7 @@ virCgroupV1GetMemoryStat(virCgroupPtr group, unsigned long long *unevictable) { int ret = -1; - char *stat = NULL; + g_autofree char *stat = NULL; char *line = NULL; unsigned long long cacheVal = 0; unsigned long long activeAnonVal = 0; @@ -1614,7 +1614,6 @@ virCgroupV1GetMemoryStat(virCgroupPtr group, ret = 0; cleanup: - VIR_FREE(stat); return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/vircgroupv1.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/vircgroupv1.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virvhba.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virvhba.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/util/virvhba.c b/src/util/virvhba.c index a80145b8fd..e0a79344cc 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c @@ -121,10 +121,10 @@ virVHBAGetConfig(const char *sysfs_prefix, sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, host, entry); if (!virFileExists(sysfs_path)) - goto cleanup; + return result; if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return result; if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -136,7 +136,6 @@ virVHBAGetConfig(const char *sysfs_prefix, result = g_strdup(p); - cleanup: return result; } @@ -206,15 +205,13 @@ virVHBAFindVportHost(const char *sysfs_prefix) if ((strlen(max_vports) >= strlen(vports)) || ((strlen(max_vports) == strlen(vports)) && strcmp(max_vports, vports) > 0)) { - ret = g_strdup(entry->d_name); - goto cleanup; + return g_strdup(entry->d_name); } VIR_FREE(max_vports); VIR_FREE(vports); } - cleanup: return ret; } @@ -248,7 +245,7 @@ virVHBAManageVport(const int parent_host, default: virReportError(VIR_ERR_OPERATION_INVALID, _("Invalid vport operation (%d)"), operation); - goto cleanup; + return ret; } operation_path = g_strdup_printf("%s/host%d/%s", SYSFS_FC_HOST_PATH, @@ -264,7 +261,7 @@ virVHBAManageVport(const int parent_host, _("vport operation '%s' is not supported " "for host%d"), operation_file, parent_host); - goto cleanup; + return ret; } } @@ -284,7 +281,6 @@ virVHBAManageVport(const int parent_host, "vport create/delete failed"), vport_name, operation_path); - cleanup: return ret; } @@ -315,12 +311,11 @@ vhbaReadCompareWWN(const char *prefix, path = g_strdup_printf("%s/%s/%s", prefix, d_name, f_name); if (!virFileExists(path)) { - ret = 0; - goto cleanup; + return 0; } if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -334,8 +329,6 @@ vhbaReadCompareWWN(const char *prefix, else ret = 1; - cleanup: - return ret; } @@ -418,7 +411,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, if ((rc = vhbaReadCompareWWN(prefix, entry->d_name, "fabric_name", fabric_wwn)) < 0) - goto cleanup; + return ret; if (rc == 0) continue; @@ -427,7 +420,6 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix, break; } - cleanup: return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virvhba.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virvhba.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/src/util/virvhba.c b/src/util/virvhba.c index a80145b8fd..e0a79344cc 100644 --- a/src/util/virvhba.c +++ b/src/util/virvhba.c @@ -121,10 +121,10 @@ virVHBAGetConfig(const char *sysfs_prefix, sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH, host, entry);
if (!virFileExists(sysfs_path)) - goto cleanup; + return result;
At this point, result is NULL so you can return it directly.
if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return result;
Same here.
if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -136,7 +136,6 @@ virVHBAGetConfig(const char *sysfs_prefix,
result = g_strdup(p);
- cleanup: return result;
and just return g_strdup(p); here to avoid the need for 'result' completely.
}
@@ -206,15 +205,13 @@ virVHBAFindVportHost(const char *sysfs_prefix) if ((strlen(max_vports) >= strlen(vports)) || ((strlen(max_vports) == strlen(vports)) && strcmp(max_vports, vports) > 0)) { - ret = g_strdup(entry->d_name); - goto cleanup; + return g_strdup(entry->d_name); }
VIR_FREE(max_vports); VIR_FREE(vports); }
- cleanup: return ret; }
return NULL;
@@ -248,7 +245,7 @@ virVHBAManageVport(const int parent_host, default: virReportError(VIR_ERR_OPERATION_INVALID, _("Invalid vport operation (%d)"), operation); - goto cleanup; + return ret;
return -1;
}
operation_path = g_strdup_printf("%s/host%d/%s", SYSFS_FC_HOST_PATH, @@ -264,7 +261,7 @@ virVHBAManageVport(const int parent_host, _("vport operation '%s' is not supported " "for host%d"), operation_file, parent_host); - goto cleanup; + return ret;
return -1;
} }
@@ -284,7 +281,6 @@ virVHBAManageVport(const int parent_host, "vport create/delete failed"), vport_name, operation_path);
- cleanup: return ret; }
@@ -315,12 +311,11 @@ vhbaReadCompareWWN(const char *prefix, path = g_strdup_printf("%s/%s/%s", prefix, d_name, f_name);
if (!virFileExists(path)) { - ret = 0; - goto cleanup; + return 0; }
if (virFileReadAll(path, 1024, &buf) < 0) - goto cleanup; + return ret;
return -1;
if ((p = strchr(buf, '\n'))) *p = '\0'; @@ -334,8 +329,6 @@ vhbaReadCompareWWN(const char *prefix, else ret = 1;
- cleanup: - return ret; }
@@ -418,7 +411,7 @@ virVHBAGetHostByFabricWWN(const char *sysfs_prefix,
if ((rc = vhbaReadCompareWWN(prefix, entry->d_name, "fabric_name", fabric_wwn)) < 0) - goto cleanup; + return ret;
return NULL; Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virutil.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virutil.c | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/src/util/virutil.c b/src/util/virutil.c index d4b864d5cb..52ab6c6e6d 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -773,7 +773,7 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret; } if (!pw) { @@ -785,15 +785,13 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) name, g_strerror(rc)); } - ret = 1; - goto cleanup; + return 1; } if (uid) *uid = pw->pw_uid; ret = 0; - cleanup: return ret; } @@ -852,7 +850,7 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret; } if (!gr) { @@ -864,16 +862,13 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) name, g_strerror(rc)); } - ret = 1; - goto cleanup; + return 1; } if (gid) *gid = gr->gr_gid; - ret = 0; - cleanup: - return ret; + return 0; } /* Try to match a group id based on `group`. The default behavior is to parse @@ -981,18 +976,17 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list) for (i = 0; i < ret; i++) { if ((*list)[i] == gid) - goto cleanup; + return ret; } if (VIR_APPEND_ELEMENT(*list, i, gid) < 0) { ret = -1; VIR_FREE(*list); - goto cleanup; + return ret; } else { ret = i; } } - cleanup: return ret; } @@ -1409,18 +1403,17 @@ virSetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret; } val = g_strdup_printf("%d", unpriv_sgio); if ((rc = virFileWriteStr(sysfs_path, val, 0)) < 0) { virReportSystemError(-rc, _("failed to set %s"), sysfs_path); - goto cleanup; + return ret; } ret = 0; - cleanup: return ret; } @@ -1440,11 +1433,11 @@ virGetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret; } if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; @@ -1452,12 +1445,10 @@ virGetDeviceUnprivSGIO(const char *path, if (virStrToLong_i(buf, NULL, 10, unpriv_sgio) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse value of %s"), sysfs_path); - goto cleanup; + return ret; } - ret = 0; - cleanup: - return ret; + return 0; } @@ -1474,7 +1465,6 @@ virGetDeviceUnprivSGIO(const char *path, int virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) { - int rc = -1; uid_t theuid; gid_t thegid; g_autofree char *tmp_label = NULL; @@ -1490,7 +1480,7 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) virReportError(VIR_ERR_INVALID_ARG, _("Failed to parse uid and gid from '%s'"), label); - goto cleanup; + return -1; } *sep = '\0'; owner = tmp_label; @@ -1501,17 +1491,14 @@ virParseOwnershipIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) */ if (virGetUserID(owner, &theuid) < 0 || virGetGroupID(group, &thegid) < 0) - goto cleanup; + return -1; if (uidPtr) *uidPtr = theuid; if (gidPtr) *gidPtr = thegid; - rc = 0; - - cleanup: - return rc; + return 0; } static time_t selfLastChanged; -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virutil.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virutil.c | 45 ++++++++++++++++----------------------------- 1 file changed, 16 insertions(+), 29 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c index d4b864d5cb..52ab6c6e6d 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -773,7 +773,7 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok)
while ((rc = getpwnam_r(name, &pwbuf, strbuf, strbuflen, &pw)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret;
return -1; That way it's obvious it always returns an error code. By using ret the reader has to track whether the variable has not been changed since its initialization. We've also had bugs in the past with reusal of the 'ret' variable - a succsesful call left it at '0', while the rest of the code assumed it was still at -1.
}
if (!pw) { @@ -785,15 +785,13 @@ virGetUserIDByName(const char *name, uid_t *uid, bool missing_ok) name, g_strerror(rc)); }
- ret = 1; - goto cleanup; + return 1; }
if (uid) *uid = pw->pw_uid;
ret = 0;
- cleanup: return ret;
return 0;
}
@@ -852,7 +850,7 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok)
while ((rc = getgrnam_r(name, &grbuf, strbuf, strbuflen, &gr)) == ERANGE) { if (VIR_RESIZE_N(strbuf, strbuflen, strbuflen, strbuflen) < 0) - goto cleanup; + return ret; }
if (!gr) { @@ -864,16 +862,13 @@ virGetGroupIDByName(const char *name, gid_t *gid, bool missing_ok) name, g_strerror(rc)); }
- ret = 1; - goto cleanup; + return 1; }
if (gid) *gid = gr->gr_gid; - ret = 0;
- cleanup: - return ret; + return 0; }
/* Try to match a group id based on `group`. The default behavior is to parse @@ -981,18 +976,17 @@ virGetGroupList(uid_t uid, gid_t gid, gid_t **list)
for (i = 0; i < ret; i++) { if ((*list)[i] == gid) - goto cleanup; + return ret; } if (VIR_APPEND_ELEMENT(*list, i, gid) < 0) {
ret = -1; VIR_FREE(*list); - goto cleanup; + return ret;
return -1; and the assingment above can be dropped.
} else { ret = i; } }
- cleanup: return ret; }
@@ -1409,18 +1403,17 @@ virSetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret;
-1
}
val = g_strdup_printf("%d", unpriv_sgio);
if ((rc = virFileWriteStr(sysfs_path, val, 0)) < 0) { virReportSystemError(-rc, _("failed to set %s"), sysfs_path); - goto cleanup; + return ret;
-1
}
ret = 0; - cleanup: return ret;
return 0; and the ret variable can be dropped
}
@@ -1440,11 +1433,11 @@ virGetDeviceUnprivSGIO(const char *path, if (!virFileExists(sysfs_path)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unpriv_sgio is not supported by this kernel")); - goto cleanup; + return ret;
-1
}
if (virFileReadAll(sysfs_path, 1024, &buf) < 0) - goto cleanup; + return ret;
-1
if ((tmp = strchr(buf, '\n'))) *tmp = '\0'; @@ -1452,12 +1445,10 @@ virGetDeviceUnprivSGIO(const char *path, if (virStrToLong_i(buf, NULL, 10, unpriv_sgio) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse value of %s"), sysfs_path); - goto cleanup; + return ret;
-1
}
- ret = 0; - cleanup: - return ret; + return 0; }
Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/viruri.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/viruri.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/viruri.c b/src/util/viruri.c index 704e5b2132..d49821451e 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -241,10 +241,9 @@ virURIFormat(virURIPtr uri) ret = (char *)xmlSaveUri(&xmluri); if (!ret) { virReportOOMError(); - goto cleanup; + return ret; } - cleanup: return ret; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/viruri.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/viruri.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/viruri.c b/src/util/viruri.c index 704e5b2132..d49821451e 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -241,10 +241,9 @@ virURIFormat(virURIPtr uri) ret = (char *)xmlSaveUri(&xmluri); if (!ret) { virReportOOMError(); - goto cleanup; + return ret;
return NULL;
}
- cleanup: return ret; }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virsysinfo.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virsysinfo.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 36317c97eb..45a950c85a 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -411,7 +411,7 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) cur = strchr(base, ':') + 1; if (VIR_EXPAND_N(ret->processor, ret->nprocessor, 1) < 0) - goto error; + return -1; processor = &ret->processor[ret->nprocessor - 1]; virSkipSpaces(&cur); @@ -425,9 +425,6 @@ virSysinfoParseARMProcessor(const char *base, virSysinfoDefPtr ret) } return 0; - - error: - return -1; } /* virSysinfoRead for ARMv7 -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virstorageencryption.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virstorageencryption.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index ed29b4cfa5..a330b79637 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -176,13 +176,12 @@ static int virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, virStorageEncryptionInfoDefPtr info) { - int ret = -1; g_autofree char *size_str = NULL; if (!(info->cipher_name = virXMLPropString(info_node, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cipher info missing 'name' attribute")); - goto cleanup; + return -1; } if ((size_str = virXMLPropString(info_node, "size")) && @@ -190,22 +189,19 @@ virStorageEncryptionInfoParseCipher(xmlNodePtr info_node, virReportError(VIR_ERR_XML_ERROR, _("cannot parse cipher size: '%s'"), size_str); - goto cleanup; + return -1; } if (!size_str) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cipher info missing 'size' attribute")); - goto cleanup; + return -1; } info->cipher_mode = virXMLPropString(info_node, "mode"); info->cipher_hash = virXMLPropString(info_node, "hash"); - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virstorageencryption.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virstorageencryption.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virsecret.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virsecret.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 78a3b41bc5..9ed803d45b 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -67,27 +67,26 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, { g_autofree char *uuid = NULL; g_autofree char *usage = NULL; - int ret = -1; uuid = virXMLPropString(secretnode, "uuid"); usage = virXMLPropString(secretnode, "usage"); if (uuid == NULL && usage == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing secret uuid or usage attribute")); - goto cleanup; + return -1; } if (uuid && usage) { virReportError(VIR_ERR_XML_ERROR, "%s", _("either secret uuid or usage expected")); - goto cleanup; + return -1; } if (uuid) { if (virUUIDParse(uuid, def->u.uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, _("invalid secret uuid '%s'"), uuid); - goto cleanup; + return -1; } def->type = VIR_SECRET_LOOKUP_TYPE_UUID; } else { @@ -95,10 +94,7 @@ virSecretLookupParseSecret(xmlNodePtr secretnode, usage = NULL; def->type = VIR_SECRET_LOOKUP_TYPE_USAGE; } - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virsecret.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virsecret.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virscsihost.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virscsihost.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/virscsihost.c b/src/util/virscsihost.c index 2ce33e4cfb..3aecb3146f 100644 --- a/src/util/virscsihost.c +++ b/src/util/virscsihost.c @@ -113,7 +113,7 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, host_link = g_strdup_printf("%s/%s", prefix, entry->d_name); if (virFileResolveLink(host_link, &host_path) < 0) - goto cleanup; + return ret; if (!strstr(host_path, parentaddr)) { VIR_FREE(host_link); @@ -131,13 +131,13 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, } if (virFileReadAll(unique_path, 1024, &buf) < 0) - goto cleanup; + return ret; if ((p = strchr(buf, '\n'))) *p = '\0'; if (virStrToLong_ui(buf, NULL, 10, &read_unique_id) < 0) - goto cleanup; + return ret; VIR_FREE(buf); @@ -150,7 +150,6 @@ virSCSIHostFindByPCI(const char *sysfs_prefix, break; } - cleanup: return ret; } @@ -231,10 +230,8 @@ virSCSIHostGetNameByParentaddr(unsigned int domain, _("Failed to find scsi_host using PCI '%s' " "and unique_id='%u'"), parentaddr, unique_id); - goto cleanup; } - cleanup: return name; } -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virrotatingfile.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virrotatingfile.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/virrotatingfile.c b/src/util/virrotatingfile.c index 6d8076e7c4..45dc66e94d 100644 --- a/src/util/virrotatingfile.c +++ b/src/util/virrotatingfile.c @@ -364,7 +364,6 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) size_t i; g_autofree char *nextpath = NULL; g_autofree char *thispath = NULL; - int ret = -1; VIR_DEBUG("Rollover %s", file->basepath); if (file->maxbackup == 0) { @@ -373,7 +372,7 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) virReportSystemError(errno, _("Unable to remove %s"), file->basepath); - goto cleanup; + return -1; } } else { nextpath = g_strdup_printf("%s.%zu", file->basepath, file->maxbackup - 1); @@ -391,7 +390,7 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) virReportSystemError(errno, _("Unable to rename %s to %s"), thispath, nextpath); - goto cleanup; + return -1; } VIR_FREE(nextpath); @@ -401,9 +400,7 @@ virRotatingFileWriterRollover(virRotatingFileWriterPtr file) VIR_DEBUG("Rollover done %s", file->basepath); - ret = 0; - cleanup: - return ret; + return 0; } -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virnetdevbandwidth.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virnetdevbandwidth.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index 364b39e3c1..e222bcd6a2 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -711,16 +711,13 @@ virNetDevBandwidthUpdateFilter(const char *ifname, const virMacAddr *ifmac_ptr, unsigned int id) { - int ret = -1; g_autofree char *class_id = NULL; class_id = g_strdup_printf("1:%x", id); if (virNetDevBandwidthManipulateFilter(ifname, ifmac_ptr, id, class_id, true, true) < 0) - goto cleanup; + return -1; return 0; - cleanup: - return ret; } -- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virlog.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virlog.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index e12fd58831..2bf606b8c5 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -528,7 +528,8 @@ virLogVMessage(virLogSourcePtr source, if (source->serial < virLogFiltersSerial) virLogSourceUpdate(source); if (priority < source->priority) - goto cleanup; + errno = saved_errno; + return; /* * serialize the error message, add level and timestamp @@ -601,8 +602,6 @@ virLogVMessage(virLogSourcePtr source, str, msg, (void *) STDERR_FILENO); } virLogUnlock(); - cleanup: - errno = saved_errno; } -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virlog.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virlog.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c index e12fd58831..2bf606b8c5 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -528,7 +528,8 @@ virLogVMessage(virLogSourcePtr source, if (source->serial < virLogFiltersSerial) virLogSourceUpdate(source); if (priority < source->priority) - goto cleanup; + errno = saved_errno; + return;
/* * serialize the error message, add level and timestamp @@ -601,8 +602,6 @@ virLogVMessage(virLogSourcePtr source, str, msg, (void *) STDERR_FILENO); } virLogUnlock(); - cleanup:
- errno = saved_errno;
This code is also executed for code paths that fall through the cleanup label, it cannot be removed.
}
-- 2.29.0

From: Barrett Schonefeld <bschoney@utexas.edu> - src/util/virdnsmasq.c Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virdnsmasq.c | 46 +++++++++++++------------------------------ 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 5f477c976d..b41cdb8047 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -168,7 +168,6 @@ addnhostsWrite(const char *path, FILE *f; bool istmp = true; size_t i, j; - int rc = 0; /* even if there are 0 hosts, create a 0 length file, to allow * for runtime addition. @@ -179,58 +178,51 @@ addnhostsWrite(const char *path, if (!(f = fopen(tmp, "w"))) { istmp = false; if (!(f = fopen(path, "w"))) { - rc = -errno; - goto cleanup; + return -errno; } } for (i = 0; i < nhosts; i++) { if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) { - rc = -errno; VIR_FORCE_FCLOSE(f); if (istmp) unlink(tmp); - goto cleanup; + return -errno; } for (j = 0; j < hosts[i].nhostnames; j++) { if (fputs(hosts[i].hostnames[j], f) == EOF || fputc('\t', f) == EOF) { - rc = -errno; VIR_FORCE_FCLOSE(f); if (istmp) unlink(tmp); - goto cleanup; + return -errno; } } if (fputc('\n', f) == EOF) { - rc = -errno; VIR_FORCE_FCLOSE(f); if (istmp) unlink(tmp); - goto cleanup; + return -errno; } } if (VIR_FCLOSE(f) == EOF) { - rc = -errno; - goto cleanup; + return -errno; } if (istmp && rename(tmp, path) < 0) { - rc = -errno; unlink(tmp); - goto cleanup; + return -errno; } - cleanup: - return rc; + return 0; } static int @@ -366,7 +358,6 @@ hostsfileWrite(const char *path, FILE *f; bool istmp = true; size_t i; - int rc = 0; /* even if there are 0 hosts, create a 0 length file, to allow * for runtime addition. @@ -377,36 +368,31 @@ hostsfileWrite(const char *path, if (!(f = fopen(tmp, "w"))) { istmp = false; if (!(f = fopen(path, "w"))) { - rc = -errno; - goto cleanup; + return -errno; } } for (i = 0; i < nhosts; i++) { if (fputs(hosts[i].host, f) == EOF || fputc('\n', f) == EOF) { - rc = -errno; VIR_FORCE_FCLOSE(f); if (istmp) unlink(tmp); - goto cleanup; + return -errno; } } if (VIR_FCLOSE(f) == EOF) { - rc = -errno; - goto cleanup; + return -errno; } if (istmp && rename(tmp, path) < 0) { - rc = -errno; unlink(tmp); - goto cleanup; + return -errno; } - cleanup: - return rc; + return 0; } static int @@ -681,16 +667,12 @@ dnsmasqCapsSetFromBuffer(dnsmasqCapsPtr caps, const char *buf) static int dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path) { - int ret = -1; g_autofree char *buf = NULL; if (virFileReadAll(path, 1024 * 1024, &buf) < 0) - goto cleanup; - - ret = dnsmasqCapsSetFromBuffer(caps, buf); + return -1; - cleanup: - return ret; + return dnsmasqCapsSetFromBuffer(caps, buf); } static int -- 2.29.0

On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virdnsmasq.c
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virdnsmasq.c | 46 +++++++++++++------------------------------ 1 file changed, 14 insertions(+), 32 deletions(-)
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index 5f477c976d..b41cdb8047 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -168,7 +168,6 @@ addnhostsWrite(const char *path, FILE *f; bool istmp = true; size_t i, j; - int rc = 0;
/* even if there are 0 hosts, create a 0 length file, to allow * for runtime addition. @@ -179,58 +178,51 @@ addnhostsWrite(const char *path, if (!(f = fopen(tmp, "w"))) { istmp = false; if (!(f = fopen(path, "w"))) { - rc = -errno; - goto cleanup; + return -errno; } }
for (i = 0; i < nhosts; i++) { if (fputs(hosts[i].ip, f) == EOF || fputc('\t', f) == EOF) { - rc = -errno; VIR_FORCE_FCLOSE(f);
While VIR_FORCE_FCLOSE saves errno so it should preserve its value
if (istmp) unlink(tmp);
that's not true for unlink, so this kind of usage of 'rc' is necessary. Jano

I've changed the commit summary to: util: xml: convert pointers to use g_autofree to distinguish this from the other commits in places where only summaries are shown. On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virxml.c
There is no need to list the changed files in the commit message, git tracks that for you.
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virxml.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On a Tuesday in 2020, Ján Tomko wrote:
I've changed the commit summary to: util: xml: convert pointers to use g_autofree to distinguish this from the other commits in places where only summaries are shown.
On a Monday in 2020, Ryan Gahagan wrote:
From: Barrett Schonefeld <bschoney@utexas.edu>
- src/util/virxml.c
There is no need to list the changed files in the commit message, git tracks that for you.
Signed-off-by: Barrett Schonefeld <bschoney@utexas.edu> --- src/util/virxml.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
I've pushed all the patches I gave a 'Reviewed-by' to. Jano
participants (2)
-
Ján Tomko
-
Ryan Gahagan