Re: [Xen-devel] [PATCH v10 09/11] vpci/msi: add MSI handlers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.03.18 at 15:34, <roger.pau@xxxxxxxxxx> wrote:
> On Fri, Mar 16, 2018 at 08:04:55AM -0600, Jan Beulich wrote:
>> >>> On 16.03.18 at 14:30, <roger.pau@xxxxxxxxxx> wrote:
>> > --- a/xen/drivers/vpci/vpci.c
>> > +++ b/xen/drivers/vpci/vpci.c
>> > @@ -47,6 +47,7 @@ void vpci_remove_device(struct pci_dev *pdev)
>> >          xfree(r);
>> >      }
>> >      spin_unlock(&pdev->vpci->lock);
>> > +    xfree(pdev->vpci->msi);
>> >      xfree(pdev->vpci);
>> 
>> Okay, the #ifdef here is gone as agreed. But something is still fishy
>> here for the test utility: You don't change the test code here at all,
>> which made me go look why that is. As it looks you use a single
>> static struct vpci instance there. That'll make the xfree() you add
>> here correct, but the other one above then is wrong (you're freeing
>> static data). Or am I missing something?
> 
> vpci_remove_device is never called from the user-space test harness,
> so it just needs to build, but not necessarily be correct in that
> context.
> 
> The test harness doesn't use vpci_add_handlers or vpci_remove_device,
> it just adds and removes handlers using vpci_add_register and
> vpci_remove_register.

I dislike such a setup - then you'd better #ifdef out the entire
function, and undo some of the other recently added #ifdef-ary.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel