Re: [libvirt PATCH 00/10] virHashNew refactorings - part VI
On Mon, 2021-07-19 at 14:08 +0200, Peter Krempa wrote:
On Mon, Jul 19, 2021 at 11:55:44 +0200, Tim Wiederhake wrote:
> "virHashNew" cannot return NULL, yet we check for NULL in various
> places.
>
> See
> https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html
> .
>
> Tim Wiederhake (10):
> qemuStateInitialize: `virHashNew` cannot return NULL
> qemuMonitorGetPRManagerInfo: `virHashNew` cannot return NULL
> qemuMonitorGetPRManagerInfo: Use automatic memory management
> qemuMonitorGetPRManagerInfo: Remove superfluous `goto`s
> qemuMonitorJSONGetAllBlockJobInfo: `virHashNew` cannot return
> NULL
> qemuMonitorJSONGetAllBlockJobInfo: Use automatic memory
> management
> qemuMonitorJSONGetAllBlockJobInfo: Remove superfluous `goto`s
> testQemuGetLatestCaps: `virHashNew` cannot return NULL
> testQemuGetLatestCaps: Use automatic memory management
> testQemuGetLatestCaps: Remove superfluous `goto`s
Series:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
For any further cleanups please consider fixing similar instances
(e.g.
removal of NULL check for virHashNew) on a per-file basis rather than
per function.
Same way in most cases where you switching to automatic memory
freeing
on a per-function basis it's okay to include cleanup of jumps/labels.
We mostly madated separation if you want to do it on a per-file
basis.
I have 38 patches still in my queue.
6 of those are two "cannot return null; use g_auto*; remove goto"
sequences that I would like to keep separate. The remaining patches are
all of the "cannot return null" kind and spread over the entire code
base. Applying them on a per-file basis would reduce the number of
patches by only four, from 32 to 28.
I am aware that this refactoring is quite noisy, but I do not think
that grouping by file makes it significantly better. What I can offer
is grouping by directory / subsystem (e.g. conf, nwfilter, qemu, util,
etc.), or sending out all remaining patches in one go, so that the
amount of mails is the same, but the process is hopefully faster.
Your thoughts?
Tim