Hi Peter,
On Fri, Nov 1, 2024 at 4:25 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Tue, 15 Oct 2024 at 09:52, <marcandre.lur...@redhat.com> wrote:
> >
> > From: Roman Penyaev <r.peni...@gmail.com>
> >
> > With bitset management now it becomes feasible to implement
> > the logic of detaching frontends from multiplexer.
> >
> > Signed-off-by: Roman Penyaev <r.peni...@gmail.com>
> > Cc: "Marc-André Lureau" <marcandre.lur...@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > Message-ID: <20241014152408.427700-8-r.peni...@gmail.com>
>
> Hi; Coverity reports an issue with this patch. I think
> it's probably a bit confused, but on the other hand
> I read the code and am also a bit confused so we could
> probably adjust it to be clearer...
>
> > +bool mux_chr_detach_frontend(MuxChardev *d, unsigned int tag)
> > +{
> > + unsigned int bit;
> > +
> > + bit = find_next_bit(&d->mux_bitset, MAX_MUX, tag);
>
> Why are we calling find_next_bit() here? Coverity
> points out that it can return MAX_MUX, which means that
> if the caller passed in MAX_MUX then we will try to
> dereference d->backends[MAX_MUX] which is off the
> end of the array.
>
> What I was expecting this code to do was to check
> "is the bit actually currently set?", which would be
> if (!(d->mux_bitset & (1 << bit))) {
> ...
> }
Yep, that looks less confusing. I'll send a follow up commit
on that.
Thanks.
--
Roman
