Re: [PATCH] scripts/coverity-scan/COMPONENTS.md: Update paths to match gitlab CI

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 31 May 2024 at 16:17, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Peter,
>
> On 31/5/24 16:21, Peter Maydell wrote:

> > If there are any other changes we want to make to our component
> > regexes, now would be a great time to suggest them, because this
> > change is going to involve "delete every existing component and
> > recreate"...
> > ---
> >   scripts/coverity-scan/COMPONENTS.md | 104 ++++++++++++++--------------
> >   1 file changed, 52 insertions(+), 52 deletions(-)
>
>
> > diff --git a/scripts/coverity-scan/COMPONENTS.md b/scripts/coverity-scan/COMPONENTS.md
> > index 1537e49cd5a..98d4bcd6a50 100644
> > --- a/scripts/coverity-scan/COMPONENTS.md
> > +++ b/scripts/coverity-scan/COMPONENTS.md
> > @@ -1,157 +1,157 @@
> >   This is the list of currently configured Coverity components:
>
>
> >   block
> > -  ~ (/qemu)?(/block.*|(/include?)/(block|storage-daemon)/.*|(/include)?/hw/(block|ide|nvme)/.*|/qemu-(img|io).*|/util/(aio|async|thread-pool).*)
> > +  ~ .*/qemu(/block.*|(/include?)/(block|storage-daemon)/.*|(/include)?/hw/(block|ide|nvme)/.*|/qemu-(img|io).*|/util/(aio|async|thread-pool).*)
>
> util/block-helpers.[ch]
>
> I'd put hw/block/ to another bucket that the block subsystem.

MAINTAINERS considers both block/ and hw/block/ to be part of the
"block layer core".

> >   char
> > -  ~ (/qemu)?(/qemu-char\.c|/include/sysemu/char\.h|(/include)?/hw/char/.*)
> > +  ~ .*/qemu(/qemu-char\.c|/include/sysemu/char\.h|(/include)?/hw/char/.*)
>
> Is 'char' the same as 'chardev'?

Good question. It looks like we don't cover chardev at all currently.
I guess we should probably prefer to track chardev separately.
qemu-char.c and include/sysemu/char.h don't exist any more.


> >   crypto
> > -  ~ (/qemu)?((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*)
> > +  ~ .*/qemu((/include)?/crypto/.*|/hw/.*/.*crypto.*|(/include/sysemu|/backends)/cryptodev.*)
>
> Maybe worth covering host/include/*/host/crypto/?

Seems reasonable.

> >   disas
> > -  ~ (/qemu)?((/include)?/disas.*)
> > +  ~ .*/qemu((/include)?/disas.*)
>
> Missing:
>
> target/avr/disas.c
> target/loongarch/disas.c
> target/openrisc/disas.c
> target/rx/disas.c

Why do these targets not put their disassembler in disas/
like all the others? We can't really catch them with a
wildcard, because the target entry earlier will match first.
I'm kind of inclined to leave these as-is, because if
somebody put the disassembler in target/ then having it
appear in the component for that target is fine for it being
basically classified to get the attention of the right person.

> >   migration
> > -  ~ (/qemu)?((/include)?/migration/.*)
> > +  ~ .*/qemu((/include)?/migration/.*)
>
> Not sure about:
>
> hw/vfio/migration.c

I think that's correctly classified under vfio.

> >   monitor
> > -  ~ (/qemu)?(/qapi.*|/qobject/.*|/monitor\..*|/[hq]mp\..*)
> > +  ~ .*/qemu(/qapi.*|/qobject/.*|/monitor\..*|/[hq]mp\..*)
>
> Apparently the pattern is now foo-[hq]mp-cmds.[ch].
>
> Not matched:
> hmp-commands-info.hx
> hmp-commands.hx

These aren't C, and because hxtool doesn't try to preserve
file-and-line-number info for them I don't think Coverity
will ever report an issue in them.

> hw/virtio/virtio-qmp.[ch]
  correctly matched under "virtio"

> include/qapi/qmp-event.h
> job-qmp.c
> monitor/qemu-config-qmp.c

Yeah, these should all be monitor.

> python/qemu/qmp/events.py
> scripts/python_qmp_updater.py

No point in caring much about the classification for non-C
files, since Coverity is only scanning C code.

> tests/unit/test-qmp-event.c
>
> Not sure about (not covered in testlibs):
>
> tests/qtest/libqmp.c
> tests/qtest/libqmp.h

Yes, we should expand testlibs to cover these.

> >   trace
> > -  ~ (/qemu)?(/.*trace.*\.[ch])
> > +  ~ .*/qemu(/.*trace.*\.[ch])
>
> >   user
> > -  ~ (/qemu)?(/linux-user/.*|/bsd-user/.*|/user-exec\.c|/thunk\.c|/include/user/.*)
> > +  ~ .*/qemu(/linux-user/.*|/bsd-user/.*|/user-exec\.c|/thunk\.c|/include/user/.*)
>
> >   xen
> > -  ~ (/qemu)?(.*/xen.*)
> > +  ~ .*/qemu(.*/xen.*)
>
> We could match .*xen.* like trace (ditto other accelerators).

That would produce mis-hits on audio/mixeng.c. The
wide match on trace actually also has mis-hits because
it matches on os-strace.h in the bsd-user subdirectories.
So I think it would be better to make the trace component
look only for trace.*\.[ch] rather than to change this.

> >   hvf
> > -  ~ (/qemu)?(.*/hvf.*)
> > +  ~ .*/qemu(.*/hvf.*)
> >
> >   kvm
> > -  ~ (/qemu)?(.*/kvm.*)
> > +  ~ .*/qemu(.*/kvm.*)
> >
> >   tcg
> > -  ~ (/qemu)?(/accel/tcg|/replay|/tcg)/.*
> > +  ~ .*/qemu(/accel/tcg|/replay|/tcg)/.*
> >
> >   sysemu
> > -  ~ (/qemu)?(/system/.*|/accel/.*)
> > +  ~ .*/qemu(/system/.*|/accel/.*)

thanks
-- PMM