Hi Alex, On 3/30/2023 3:54 PM, Alex Williamson wrote: > On Thu, 30 Mar 2023 15:32:20 -0700 > Reinette Chatre <reinette.chatre@xxxxxxxxx> wrote: >> On 3/30/2023 1:26 PM, Alex Williamson wrote: >>> On Tue, 28 Mar 2023 14:53:29 -0700 >>> Reinette Chatre <reinette.chatre@xxxxxxxxx> wrote: >> ... >> >>>> @@ -399,7 +399,8 @@ static int vfio_msi_set_vector_signal(struct vfio_pci_core_device *vdev, >>>> static int vfio_msi_set_block(struct vfio_pci_core_device *vdev, unsigned start, >>>> unsigned count, int32_t *fds, bool msix) >>>> { >>>> - int i, j, ret = 0; >>>> + int i, ret = 0; >>>> + unsigned int j; >>>> >>>> if (start >= vdev->num_ctx || start + count > vdev->num_ctx) >>>> return -EINVAL; >>> >>> Unfortunately this turns the unwind portion of the function into an >>> infinite loop in the common case when @start is zero: >>> >>> for (--j; j >= (int)start; j--) >>> vfio_msi_set_vector_signal(vdev, j, -1, msix); >>> >>> >> >> Thank you very much for catching this. It is not clear to me how you >> would prefer to resolve this. Would you prefer that the vector parameter >> in vfio_msi_set_vector_signal() continue to be an int and this patch be >> dropped and the "if (vector < 0)" check remains (option A)? Or, alternatively, >> I see two other possible solutions where the vector parameter in >> vfio_msi_set_vector_signal() becomes an unsigned int and the above snippet >> could be one of: >> >> option B: >> vfio_msi_set_block() >> { >> int i, j, ret = 0; >> >> ... >> for (--j; j >= (int)start; j--) >> vfio_msi_set_vector_signal(vdev, (unsigned int)j, -1, msix); >> } >> >> option C: >> vfio_msi_set_block() >> { >> int i, ret = 0; >> unsigned int j; >> >> ... >> for (--j; j >= start && j < start + count; j--) >> vfio_msi_set_vector_signal(vdev, j, -1, msix); >> } >> >> What would you prefer? > > > Hmm, C is fine, it avoids casting. I think we could also do: > > unsigned int i, j; > int ret = 0; > > ... > > for (i = start; i < j; i++) > vfio_msi_set_vector_signal(vdev, i, -1, msix); > Much better. Will do. Thank you very much. Reinette