[libvirt] [PATCH v4 0/4] qemu_process: Glib sponsored cleanup

[Cole Robinson]

On 12/19/19 4:09 PM, Daniel Henrique Barboza wrote:
> This series got buried a few months ago. Let's go onward unto
> the 20s with no one left behind.
> 
> changes from version 3 [1]:
> - rebased to master at commit 330b556829
> - removed former patch 4. The 'g_strdup_printf' change was
> made along the road
> - new patch 4: I am sending this one in separate to patch 3
> because this unneeded label was already in the code, being
> unrelated to the changes of this series. The maintainer is
> welcome to squash it into patch 3.
> 
> [1] https://www.redhat.com/archives/libvir-list/2019-October/msg01080.html
> 
> Daniel Henrique Barboza (4):
>   qemu_process.c: use g_autofree
>   qemu_process.c: use g_autoptr()
>   qemu_process.c: remove cleanup labels after g_auto*() changes
>   qemu_process.c: remove 'cleanup' label from
>     qemuProcessCreatePretendCmd()
> 
>  src/qemu/qemu_process.c | 429 +++++++++++++++-------------------------
>  1 file changed, 157 insertions(+), 272 deletions(-)
> 

Patch 3 and 4 look fine, some comments on the first two.

I can't really decide what is the best way to approach cleanups like
this. Should patches be split by function, and do all cleanups in one
pass, or do one type of cleanup, but over a larger surface per file?
Like you have done here.

The first method is more tedious, but it's easier for reviewers, and
good patches can go in first while patches with issues can be kicked out
for v2. But it could be thousands of patches judging by the 3000+
cleanup labels we have in the code base, which sounds extreme.

I think in general you will find the list more receptive to series of
small per function patches though. Optimizing for ease of review will
always give things a better chance of getting committed in my
experience. This is just recommendation for future series though, I'll
review this one as is

- Cole