[PATCH] util: convert char pointers to use g_autofree

[Barrett J Schonefeld]

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 <laine at redhat.com> 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.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201123/a120d31b/attachment-0001.htm>