Re: [PATCH] hw/nvme: use GPtrArray for blocker_features

Am Di., 28. Juli 2026 um 09:43 Uhr schrieb Klaus Jensen <[email protected]>:
>
> On Jul  9 11:01, Alexander Mikhalitsyn wrote:
> > Am Do., 9. Juli 2026 um 10:58 Uhr schrieb Peter Maydell
> > <[email protected]>:
> > >
> > > On Thu, 9 Jul 2026 at 09:53, Alexander Mikhalitsyn
> > > <[email protected]> wrote:
> > > >
> > > > Am Do., 9. Juli 2026 um 10:51 Uhr schrieb Peter Maydell
> > > > <[email protected]>:
> > > > >
> > > > > On Thu, 9 Jul 2026 at 09:47, Alexander Mikhalitsyn
> > > > > <[email protected]> wrote:
> > > > > >
> > > > > > From: Alexander Mikhalitsyn <[email protected]>
> > > > > >
> > > > > > Let's use GPtrArray to build a list of blocker features and then
> > > > > > g_strjoinv() to build a final comma-delimited string.
> > > > > >
> > > > > > While previous approach was technically correct, it is fragile
> > > > > > (because we need to take care of static buffer size choice) and
> > > > > > Coverity dislikes it too.
> > > > > >
> > > > > > Note, that we use g_ptr_array_new() to allocate array which means
> > > > > > that GDestroyNotify callback is not set, so we can pass pointers to
> > > > > > a static memory like g_ptr_array_add(..., (gpointer) "SR-IOV") without
> > > > > > any problems as there won't be any attempt to free that memory.
> > > > > >
> > > > > > Resolves: Coverity CID 1663673
> > > > > > Suggested-by: Peter Maydell <[email protected]>
> > > > > > Signed-off-by: Alexander Mikhalitsyn <[email protected]>
> > > > > > ---
> > >
> > > > > > @@ -9416,15 +9405,15 @@ static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev,
> > > > > >              }
> > > > > >
> > > > > >              if (namespaces_num > 1) {
> > > > > > -                nvme_add_blocker_feature(blocker_features,
> > > > > > -                                         "Namespace Attachment");
> > > > > > +                g_ptr_array_add(blocker_features,
> > > > > > +                                         (gpointer) "Namespace Attachment");
> > > > >
> > > >
> > > > Hi Peter,
> > > >
> > > > > Is the cast here because we're dropping the const property of the
> > > > > literal string?
> > > >
> > > > yep.
> > >
> > > Mmm. Unfortunate but unavoidable I guess.
> > >
> > > The glib docs recommend using "void *" instead of "gpointer" in
> > > new code: https://docs.gtk.org/glib/types.html#gpointer
> > > but either way
> >
> > ah, I didn't know that. My first intention was to use "void *", but
> > then I searched over QEMU codebase
> > and found a few gpointer casts in exact use-case (with g_ptr_array_add).
> >
> > I can correct this and resend if you want ;)
> >
> > >
> > > Reviewed-by: Peter Maydell <[email protected]>
> > >
> > > -- PMM
> >
>
> Thanks,
>
> Added to nvme.next with the gpointer cast changed to void *.

Thank you, Klaus ;-)

>
> Reviewed-by: Klaus Jensen <[email protected]>