Re: [PATCH] util: convert char pointers to use g_autofree
Updated the patch with the suggested changes. The last commit for the new patch is https://www.redhat.com/archives/libvir-list/2020-November/msg01324.html. On Fri, Nov 20, 2020 at 2:55 PM Laine Stump wrote: > On 11/20/20 11:43 AM, Barrett J Schonefeld wrote: > > I appreciate the feedback on this patch! > > > > I will work on splitting this into multiple patches. I believe this will > > involve redoing much of the work because I will need to split this patch > > (a single commit) into many commits. > > One suggestion on how to more easily split one patch into multiple > patches (keeping in mind there's probably a much cleaner way of doing > the same thing; this is just how I've evolved to do it): > > 1) make a new branch "X-v2" based off the branch "X" that has this > current patch > > 2) "git reset HEAD^" on the new branch - this will remove the last > commit from git, but leave the working copies of the file unchanged. > > 3) use a tool like "git meld" to interactively go through all the > changes (hunks) you've made in this single commit, *un*doing the ones > that aren't related to basic g_autofree conversion. > > 4) git add / git commit -m"convert to g_autofree" > > 5) "git meld X" to compare the original commit on the *old* branch to > the tip of the new branch, interactively re-applying all the hunks that > you had just removed. > > 5) git add / git commit -m"remove unnecessary cleanup labels and return > variables" > > > Hence, I'd like to get some > > confirmation on how I should approach the patch. > > > > I plan to: > > > > 1. Address the feedback on returning `-errno`, `0`, `-1`, etc. directly > > instead of setting the local variable, `ret`, and returning `ret`. > > 2. Submit a patch per file with only the g_autofree changes. > > 3. Submit a patch per file that removes the cleanup sections. > > A couple of qualifiers: > > a) changing the return values to constants will of course happen as a > part of the "cleanup label removal" patch (item (3)), not on its own. > > b) a recent patch from jtomko reminded me of two occasions when you > *would* want a separate patch for the g_autofree changed in a single > function all by itself: > >ii) if that change fixes a bug (which would usually be a memory leak), > > and/or > >iii) if it requires any change in the logic of the function beyond > simply adding "g_autofree virBlahPtr blah = NULL;" and > removing the VIR_FREE(blah); at the end of the scope. > > Both of these would warrant extra explanation in the commit log, and > that's easier to follow if it's isolated. > > >
Re: [PATCH] util: convert char pointers to use g_autofree
On 11/20/20 11:43 AM, Barrett J Schonefeld wrote: I appreciate the feedback on this patch! I will work on splitting this into multiple patches. I believe this will involve redoing much of the work because I will need to split this patch (a single commit) into many commits. One suggestion on how to more easily split one patch into multiple patches (keeping in mind there's probably a much cleaner way of doing the same thing; this is just how I've evolved to do it): 1) make a new branch "X-v2" based off the branch "X" that has this current patch 2) "git reset HEAD^" on the new branch - this will remove the last commit from git, but leave the working copies of the file unchanged. 3) use a tool like "git meld" to interactively go through all the changes (hunks) you've made in this single commit, *un*doing the ones that aren't related to basic g_autofree conversion. 4) git add / git commit -m"convert to g_autofree" 5) "git meld X" to compare the original commit on the *old* branch to the tip of the new branch, interactively re-applying all the hunks that you had just removed. 5) git add / git commit -m"remove unnecessary cleanup labels and return variables" Hence, I'd like to get some confirmation on how I should approach the patch. I plan to: 1. Address the feedback on returning `-errno`, `0`, `-1`, etc. directly instead of setting the local variable, `ret`, and returning `ret`. 2. Submit a patch per file with only the g_autofree changes. 3. Submit a patch per file that removes the cleanup sections. A couple of qualifiers: a) changing the return values to constants will of course happen as a part of the "cleanup label removal" patch (item (3)), not on its own. b) a recent patch from jtomko reminded me of two occasions when you *would* want a separate patch for the g_autofree changed in a single function all by itself: ii) if that change fixes a bug (which would usually be a memory leak), and/or iii) if it requires any change in the logic of the function beyond simply adding "g_autofree virBlahPtr blah = NULL;" and removing the VIR_FREE(blah); at the end of the scope. Both of these would warrant extra explanation in the commit log, and that's easier to follow if it's isolated.
Re: [PATCH] util: convert char pointers to use g_autofree
I appreciate the feedback on this patch!
I will work on splitting this into multiple patches. I believe this will
involve redoing much of the work because I will need to split this patch (a
single commit) into many commits. Hence, I'd like to get some confirmation
on how I should approach the patch.
I plan to:
1. Address the feedback on returning `-errno`, `0`, `-1`, etc. directly
instead of setting the local variable, `ret`, and returning `ret`.
2. Submit a patch per file with only the g_autofree changes.
3. Submit a patch per file that removes the cleanup sections.
On Thu, Nov 19, 2020 at 9:57 AM Laine Stump wrote:
> On 11/19/20 6:46 AM, Tim Wiederhake wrote:
> > On Wed, 2020-11-18 at 16:47 -0600, Ryan Gahagan wrote:
> >> From: Barrett Schonefeld
> >>
> >> additional conversions to the GLib API in src/util per issue #11.
> >>
> >> [...]
>
> >> return ret;
> >> }
> >>
> > I believe you can remove "cleanup" and "ret" as well. More instances
> > below.
>
>
> The method recommended to me by a few reviewers was to make *only* the
> conversions to g_autofree in one patch, then have a separate patch that
> short-circuits the "goto cleanup"s and removes the cleanup: label from
> those functions that now have an "empty" cleanup. This makes review more
> mechanical, and thus easier to verify during review.
>
>
> >
> >> diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
> >> index 9030f9218a..93bc4a129f 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;
> >> @@ -180,7 +180,7 @@ addnhostsWrite(const char *path,
> >> istmp = false;
> >> if (!(f = fopen(path, "w"))) {
> >> rc = -errno;
> >> -goto cleanup;
> >> +return rc;
> > Why not "return -errno;"? More instances below.
>
>
> Yeah, that's another that would be done in the 2nd "remove cleanup:
> labels" patch - usually you end up with all the returns just directly
> returning some value (usually 0 or -1, but it could be something like
> -errno or some other variable), and that very often makes the variable
> "ret" (or sometimes, as in this case, "rc") unused, so it would be
> removed as a part of the same patch.
>
>
> >
> > You also might want to split the patch up, e.g. go function by
> > function, to create self-contained changes.
>
>
> My opinion is that for a mechanical change like this, a separate patch
> for each function is overkill. And I'm on the fence about even having a
> separate patch for each file (it depends on how many chunks there are
> and how similar/simple the chunks are). Having several identical changes
> in one patch makes it easier to scan through all the chunks without
> needing to hop from one message to the next (and then either give a
> blanket R-b: to the series, or else reply to every single one of the
> tiny patches).
>
>
> (The downside to including too many pieces of too many files in one
> patch is that if some other unrelated future patch needs to be
> backported to a distro's stable maintenance branch of libvirt, and that
> patch created a merge conflict if the patch you made hadn't also been
> backported to the branch, then the maintainer would either need to
> backport one large patch rather than one small patch. In the case of
> g_autofree conversion patches )and other similar simple changes), I'd
> say backporting the larger patch would create any extra problems, but
> then I don't backport a lot of patches to downstream branches :-)
>
>
Re: [PATCH] util: convert char pointers to use g_autofree
On 11/19/20 6:46 AM, Tim Wiederhake wrote:
On Wed, 2020-11-18 at 16:47 -0600, Ryan Gahagan wrote:
From: Barrett Schonefeld
additional conversions to the GLib API in src/util per issue #11.
[...]
return ret;
}
I believe you can remove "cleanup" and "ret" as well. More instances
below.
The method recommended to me by a few reviewers was to make *only* the
conversions to g_autofree in one patch, then have a separate patch that
short-circuits the "goto cleanup"s and removes the cleanup: label from
those functions that now have an "empty" cleanup. This makes review more
mechanical, and thus easier to verify during review.
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 9030f9218a..93bc4a129f 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;
@@ -180,7 +180,7 @@ addnhostsWrite(const char *path,
istmp = false;
if (!(f = fopen(path, "w"))) {
rc = -errno;
-goto cleanup;
+return rc;
Why not "return -errno;"? More instances below.
Yeah, that's another that would be done in the 2nd "remove cleanup:
labels" patch - usually you end up with all the returns just directly
returning some value (usually 0 or -1, but it could be something like
-errno or some other variable), and that very often makes the variable
"ret" (or sometimes, as in this case, "rc") unused, so it would be
removed as a part of the same patch.
You also might want to split the patch up, e.g. go function by
function, to create self-contained changes.
My opinion is that for a mechanical change like this, a separate patch
for each function is overkill. And I'm on the fence about even having a
separate patch for each file (it depends on how many chunks there are
and how similar/simple the chunks are). Having several identical changes
in one patch makes it easier to scan through all the chunks without
needing to hop from one message to the next (and then either give a
blanket R-b: to the series, or else reply to every single one of the
tiny patches).
(The downside to including too many pieces of too many files in one
patch is that if some other unrelated future patch needs to be
backported to a distro's stable maintenance branch of libvirt, and that
patch created a merge conflict if the patch you made hadn't also been
backported to the branch, then the maintainer would either need to
backport one large patch rather than one small patch. In the case of
g_autofree conversion patches )and other similar simple changes), I'd
say backporting the larger patch would create any extra problems, but
then I don't backport a lot of patches to downstream branches :-)
Re: [PATCH] util: convert char pointers to use g_autofree
On 11/18/20 9:40 PM, Barrett J Schonefeld wrote: I spent a significant chunk of time trying to get `git send-email` working but struggled to get the tool to work on my computer. Is there an error message you're seeing? Is your SMTP server blocking connection? Some use gmail to work around too restrictive SMTP servers. There is even a section in git-send-email(1) man page covering how to set gmail as SMTP server. I instead used the `Email Patch` feature in GitLab to format the patch as an email. I'm surprised GitLab mangles patches like that if it has such feature. Since the formatting is wrong, I may have someone submit the patch on my behalf. Yes, that works too. Michal
Re: [PATCH] util: convert char pointers to use g_autofree
On Wed, 2020-11-18 at 16:47 -0600, Ryan Gahagan wrote:
> From: Barrett Schonefeld
>
> additional conversions to the GLib API in src/util per issue #11.
>
> Related issue: https://gitlab.com/libvirt/libvirt/-/issues/11
>
> Signed-off-by: Barrett Schonefeld
> ---
> src/util/vircgroupv1.c | 3 +-
> src/util/virdnsmasq.c| 43 -
> src/util/virfile.c | 9 ++---
> src/util/virhostcpu.c| 4 +-
> src/util/virlockspace.c | 6 +--
> src/util/virlog.c| 12 ++
> src/util/virmacmap.c | 3 +-
> src/util/virnetdevbandwidth.c| 48 ---
> src/util/virresctrl.c| 25
> src/util/virrotatingfile.c | 11 ++
> src/util/virscsihost.c | 25 +---
> src/util/virsecret.c | 14 +++
> src/util/virstorageencryption.c | 19 +++--
> src/util/virstoragefilebackend.c | 4 +-
> src/util/virsysinfo.c| 18 +++--
> src/util/viruri.c| 7 +---
> src/util/virutil.c | 66 +++---
> --
> src/util/virvhba.c | 57 ++-
> src/util/virxml.c| 7 +---
> 19 files changed, 130 insertions(+), 251 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;
> }
>
I believe you can remove "cleanup" and "ret" as well. More instances
below.
> diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
> index 9030f9218a..93bc4a129f 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;
> @@ -180,7 +180,7 @@ addnhostsWrite(const char *path,
> istmp = false;
> if (!(f = fopen(path, "w"))) {
> rc = -errno;
> -goto cleanup;
> +return rc;
Why not "return -errno;"? More instances below.
You also might want to split the patch up, e.g. go function by
function, to create self-contained changes.
Cheers,
Tim
> }
> }
>
> @@ -192,7 +192,7 @@ addnhostsWrite(const char *path,
> if (istmp)
> unlink(tmp);
>
> -goto cleanup;
> +return rc;
> }
>
> for (j = 0; j < hosts[i].nhostnames; j++) {
> @@ -203,7 +203,7 @@ addnhostsWrite(const char *path,
> if (istmp)
> unlink(tmp);
>
> -goto cleanup;
> +return rc;
> }
> }
>
> @@ -214,24 +214,21 @@ addnhostsWrite(const char *path,
> if (istmp)
> unlink(tmp);
>
> -goto cleanup;
> +return rc;
> }
> }
>
> if (VIR_FCLOSE(f) == EOF) {
> rc = -errno;
> -goto cleanup;
> +return rc;
> }
>
> if (istmp && rename(tmp, path) < 0) {
> rc = -errno;
> unlink(tmp);
> -goto cleanup;
> +return rc;
> }
>
> - cleanup:
> -VIR_FREE(tmp);
> -
> return rc;
> }
>
> @@ -364,7 +361,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;
> @@ -380,7 +377,7 @@ hostsfileWrite(const char *path,
> istmp = false;
> if (!(f = fopen(path, "w"))) {
> rc = -errno;
> -goto cleanup;
> +return rc;
> }
> }
>
> @@ -392,24 +389,21 @@ hostsfileWrite(const char *path,
> if (istmp)
> unlink(tmp);
>
> -goto cleanup;
> +return rc;
> }
> }
>
> if (VIR_FCLOSE(f) == EOF) {
> rc = -errno;
> -goto cleanup;
> +return rc;
> }
>
> if (istmp && rename(tmp, path) < 0) {
> rc = -errno;
> unlink(tmp);
> -goto cleanup;
> +return rc;
> }
>
> - cleanup:
> -VIR_FREE(tmp);
> -
> return rc;
> }
>
> @@ -686,15 +680,13 @@ static int
> dnsmasqCapsSetFromFile(dnsmasqCapsPtr caps, const char *path)
> {
>
Re: [PATCH] util: convert char pointers to use g_autofree
I spent a significant chunk of time trying to get `git send-email` working but struggled to get the tool to work on my computer. I instead used the `Email Patch` feature in GitLab to format the patch as an email. Since the formatting is wrong, I may have someone submit the patch on my behalf. On Wed, Nov 18, 2020 at 11:46 AM Michal Privoznik wrote: > On 11/17/20 9:45 PM, Barrett J Schonefeld wrote: > >>From 82f992c7ff4ef59682f42c863fca242dd84208f9 Mon Sep 17 00:00:00 2001 > > From: Barrett Schonefeld > > Date: Mon, 9 Nov 2020 16:07:25 -0600 > > Subject: [PATCH] util: convert char pointers to use g_autofree > > > > additional conversions to the GLib API in src/util per issue #11. > > > > Related issue: https://gitlab.com/libvirt/libvirt/-/issues/11 > > > > Signed-off-by: Barrett Schonefeld > > --- > > src/util/vircgroupv1.c | 3 +- > > src/util/virdnsmasq.c| 43 - > > src/util/virfile.c | 9 ++--- > > src/util/virhostcpu.c| 4 +- > > src/util/virlockspace.c | 6 +-- > > src/util/virlog.c| 12 ++ > > src/util/virmacmap.c | 3 +- > > src/util/virnetdevbandwidth.c| 48 --- > > src/util/virresctrl.c| 25 > > src/util/virrotatingfile.c | 11 ++ > > src/util/virscsihost.c | 25 +--- > > src/util/virsecret.c | 14 +++ > > src/util/virstorageencryption.c | 19 +++-- > > src/util/virstoragefilebackend.c | 4 +- > > src/util/virsysinfo.c| 18 +++-- > > src/util/viruri.c| 7 +--- > > src/util/virutil.c | 66 +++- > > src/util/virvhba.c | 57 ++- > > src/util/virxml.c| 7 +--- > > 19 files changed, 130 insertions(+), 251 deletions(-) > > > I'm sorry, I can't apply this patch, it is corrupted. Looks like you've > wrapped lines. Does 'git send-email' not work for you? Because that is > the recommended way to send patches. > > https://libvirt.org/submitting-patches.html > > Michal > >
Re: [PATCH] util: convert char pointers to use g_autofree
On 11/17/20 9:45 PM, Barrett J Schonefeld wrote: From 82f992c7ff4ef59682f42c863fca242dd84208f9 Mon Sep 17 00:00:00 2001 From: Barrett Schonefeld Date: Mon, 9 Nov 2020 16:07:25 -0600 Subject: [PATCH] util: convert char pointers to use g_autofree additional conversions to the GLib API in src/util per issue #11. Related issue: https://gitlab.com/libvirt/libvirt/-/issues/11 Signed-off-by: Barrett Schonefeld --- src/util/vircgroupv1.c | 3 +- src/util/virdnsmasq.c| 43 - src/util/virfile.c | 9 ++--- src/util/virhostcpu.c| 4 +- src/util/virlockspace.c | 6 +-- src/util/virlog.c| 12 ++ src/util/virmacmap.c | 3 +- src/util/virnetdevbandwidth.c| 48 --- src/util/virresctrl.c| 25 src/util/virrotatingfile.c | 11 ++ src/util/virscsihost.c | 25 +--- src/util/virsecret.c | 14 +++ src/util/virstorageencryption.c | 19 +++-- src/util/virstoragefilebackend.c | 4 +- src/util/virsysinfo.c| 18 +++-- src/util/viruri.c| 7 +--- src/util/virutil.c | 66 +++- src/util/virvhba.c | 57 ++- src/util/virxml.c| 7 +--- 19 files changed, 130 insertions(+), 251 deletions(-) I'm sorry, I can't apply this patch, it is corrupted. Looks like you've wrapped lines. Does 'git send-email' not work for you? Because that is the recommended way to send patches. https://libvirt.org/submitting-patches.html Michal
Re: [PATCH] util: convert char pointers to use g_autofree
On Mon, Nov 09, 2020 at 04:34:22PM -0600, Ryan Gahagan wrote:
> From: Barrett Schonefeld
>
> additional conversions to the GLib API in src/util per issue #11.
>
> files updated are:
> - src/util/vircgroupv1.c
> - src/util/virhostcpu.c
> - src/util/virlockspace.c
> - src/util/virmacmap.c
> - src/util/virresctrl.c
> - src/util/virsysinfo.c
>
> Related issue: https://gitlab.com/libvirt/libvirt/-/issues/11
>
> Signed-off-by: bschoney
> Signed-off-by: Barrett Schonefeld
> ---
> src/util/vircgroupv1.c | 3 +--
> src/util/virhostcpu.c | 4 +---
> src/util/virlockspace.c | 6 ++
> src/util/virmacmap.c| 3 +--
> src/util/virresctrl.c | 25 -
> src/util/virsysinfo.c | 9 +++--
> 6 files changed, 16 insertions(+), 34 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;
Usually the whole exercise of using g_autofree or g_autoptr is to
simplify the code by removing all the `goto` and cleanup labels
where it is possible.
Otherwise looks good.
Pavel
signature.asc
Description: PGP signature
Re: [PATCH] util: convert char pointers to use g_autofree
On Tue, Nov 10, 2020 at 6:36 AM Ryan Gahagan wrote:
> From: Barrett Schonefeld
>
> additional conversions to the GLib API in src/util per issue #11.
>
> files updated are:
> - src/util/vircgroupv1.c
> - src/util/virhostcpu.c
> - src/util/virlockspace.c
> - src/util/virmacmap.c
> - src/util/virresctrl.c
> - src/util/virsysinfo.c
>
> Related issue: https://gitlab.com/libvirt/libvirt/-/issues/11
>
> Signed-off-by: bschoney
> Signed-off-by: Barrett Schonefeld
> ---
> src/util/vircgroupv1.c | 3 +--
> src/util/virhostcpu.c | 4 +---
> src/util/virlockspace.c | 6 ++
> src/util/virmacmap.c| 3 +--
> src/util/virresctrl.c | 25 -
> src/util/virsysinfo.c | 9 +++--
> 6 files changed, 16 insertions(+), 34 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;
> }
>
> 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;
> }
>
> 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(>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(>lock);
> -VIR_FREE(respath);
> return ret;
> }
>
> 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;
> }
> 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(,
> SYSFS_RESCTRL_PATH "/%s/schemata",
> groupname);
> @@ -1753,11 +1752,9 @@ virResctrlAllocGetGroup(virResctrlInfoPtr resctrl,
> if
