Message ID | 20220111012622.19447-1-yang.lee@linux.alibaba.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | [-next] PCI: hv: Unsigned comparison with less than zero | expand |
[+cc Sunil] On Tue, Jan 11, 2022 at 09:26:22AM +0800, Yang Li wrote: > The return from the call to bitmap_find_free_region() is int, it can be > a negative error code, however this is being assigned to an unsigned > int variable 'index', so making 'index' an int. > > Eliminate the following coccicheck warning: > ./drivers/pci/controller/pci-hyperv.c:712:5-10: WARNING: Unsigned > expression compared with zero: index < 0 Definitely looks like a bug. Thanks very much for catching it! Minor things: 1) Can you make the subject and commit log talk about the *bug* this fixes, which is that an error return from bitmap_find_free_region() is currently ignored and we instead return a completely bogus *hwirq from hv_pci_vec_alloc_device_irq()? The warning is only secondary. 2) When fixing a bug, can you also mention the commit that *introduced* the bug, so we can figure out where the fix needs to be backported? It looks like c10bdb758ca4 ("PCI: hv: Add arm64 Hyper-V vPCI support"), so we should have a line like this: Fixes: c10bdb758ca4 ("PCI: hv: Add arm64 Hyper-V vPCI support") In this case, c10bdb758ca4 is still pending for the current merge window, so we'll probably squash this fix into the commit so there's no bisection window between c10bdb758ca4 and the fix. 3) Please cc the author of the original commit in case there's something we're missing (I added Sunil here). 4) Make the subject line start with a verb so it matches the style in drivers/pci/, where I try to make the subject line a sentence that makes sense all by itself and tells what the patch does. Thanks again for the fix! Bjorn > Reported-by: Abaci Robot <abaci@linux.alibaba.com> > Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> > --- > drivers/pci/controller/pci-hyperv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > index 26c9c8ec0989..20ea2ee330b8 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -701,7 +701,7 @@ static int hv_pci_vec_alloc_device_irq(struct irq_domain *domain, > irq_hw_number_t *hwirq) > { > struct hv_pci_chip_data *chip_data = domain->host_data; > - unsigned int index; > + int index; > > /* Find and allocate region from the SPI bitmap */ > mutex_lock(&chip_data->map_lock); > -- > 2.20.1.7.g153144c >
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index 26c9c8ec0989..20ea2ee330b8 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -701,7 +701,7 @@ static int hv_pci_vec_alloc_device_irq(struct irq_domain *domain, irq_hw_number_t *hwirq) { struct hv_pci_chip_data *chip_data = domain->host_data; - unsigned int index; + int index; /* Find and allocate region from the SPI bitmap */ mutex_lock(&chip_data->map_lock);
The return from the call to bitmap_find_free_region() is int, it can be a negative error code, however this is being assigned to an unsigned int variable 'index', so making 'index' an int. Eliminate the following coccicheck warning: ./drivers/pci/controller/pci-hyperv.c:712:5-10: WARNING: Unsigned expression compared with zero: index < 0 Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> --- drivers/pci/controller/pci-hyperv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)