[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 07/11] vfio/platform: Remove dead assignment in vfio_intp_int
From: |
Auger Eric |
Subject: |
Re: [PATCH 07/11] vfio/platform: Remove dead assignment in vfio_intp_interrupt() |
Date: |
Thu, 13 Aug 2020 20:02:45 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
Hi Alex,
On 8/13/20 6:59 PM, Alex Williamson wrote:
> On Thu, 13 Aug 2020 15:37:08 +0800
> Chen Qun <kuhn.chenqun@huawei.com> wrote:
>
>> Clang static code analyzer show warning:
>> hw/vfio/platform.c:239:9: warning: Value stored to 'ret' is never read
>> ret = event_notifier_test_and_clear(intp->interrupt);
>> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>> ---
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Cc: Eric Auger <eric.auger@redhat.com>
>> ---
>> hw/vfio/platform.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
>> index ac2cefc9b1..869ed2c39d 100644
>> --- a/hw/vfio/platform.c
>> +++ b/hw/vfio/platform.c
>> @@ -236,7 +236,7 @@ static void vfio_intp_interrupt(VFIOINTp *intp)
>> trace_vfio_intp_interrupt_set_pending(intp->pin);
>> QSIMPLEQ_INSERT_TAIL(&vdev->pending_intp_queue,
>> intp, pqnext);
>> - ret = event_notifier_test_and_clear(intp->interrupt);
>> + event_notifier_test_and_clear(intp->interrupt);
>> return;
>> }
>
> Testing that an event is pending in our notifier is generally a
> prerequisite to doing anything in the interrupt handler, I don't
> understand why we're just consuming it and ignoring the return value.
> The above is in the delayed handling branch of the function, but the
> normal non-delayed path would only go on to error_report() if the
> notifier is not pending and then inject an interrupt anyway. This all
> seems rather suspicious and it's a unique pattern among the vfio
> callers of this function. Is there a more fundamental bug that this
> function should perform this test once and return without doing
> anything if it's called spuriously, ie. without a notifier pending?
> Thanks,
Hum that's correct that other VFIO call sites do the check. My
understanding was that this could not fail in this case as, if we
entered the handler there was something to be cleared. In which
situation can this fail?
Thanks
Eric
>
> Alex
>
>
[PATCH 05/11] hw/virtio/vhost-user:Remove dead assignment in scrub_shadow_regions(), Chen Qun, 2020/08/13
[PATCH 09/11] usb/bus: Remove dead assignment in usb_get_fw_dev_path(), Chen Qun, 2020/08/13
[PATCH 01/11] hw/arm/virt-acpi-build:Remove dead assignment in build_madt(), Chen Qun, 2020/08/13