[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/3] Use g_new() & friends where that makes obvious sense



On Mon, Mar 14, 2022 at 05:52:32PM +0100, Markus Armbruster wrote:
> Peter Maydell <peter.maydell@xxxxxxxxxx> writes:
> 
> > On Mon, 14 Mar 2022 at 16:01, Markus Armbruster <armbru@xxxxxxxxxx> wrote:
> >>
> >> g_new(T, n) is neater than g_malloc(sizeof(T) * n).  It's also safer,
> >> for two reasons.  One, it catches multiplication overflowing size_t.
> >> Two, it returns T * rather than void *, which lets the compiler catch
> >> more type errors.
> >>
> >> This commit only touches allocations with size arguments of the form
> >> sizeof(T).
> >>
> >> Patch created mechanically with:
> >>
> >>     $ spatch --in-place --sp-file scripts/coccinelle/use-g_new-etc.cocci \
> >>              --macro-file scripts/cocci-macro-file.h FILES...
> >>
> >> Signed-off-by: Markus Armbruster <armbru@xxxxxxxxxx>
> >> ---
> >
> >>  104 files changed, 197 insertions(+), 202 deletions(-)
> >
> > I'm not going to say you must split this patch up. I'm just going to
> > say that I personally am not looking at it, because it's too big
> > for me to deal with.
> 
> As with all big but trivial Coccinelle patches, reviewing the Coccinelle
> script and a reasonably representative sample of its output is almost
> certainly a better use of reviewer time than attempting to get all the
> patches reviewed.  They are mind-numbingly dull!
> 
> For what it's worth, we've used this script several times before.  Last
> in commit bdd81addf4.

This Coccinelle is simple enough to understand, that I'd suggest that
once we merge the Coccinelle script itself, then for ongoing usage,
its output can be considered effectively pre-reviewed.

The reviewer can just re-run the Coccinelle script themselves to prove
it has the same output as the submitter claims, to validate no manual
changes are hidden in the middle of the automated patch.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.