Messages in this thread | | | Date | Fri, 15 Apr 2022 00:51:34 -0300 | Subject | Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() | From | Murilo Opsfelder Araújo <> |
| |
On 4/14/22 23:30, Murilo Opsfelder Araujo wrote: > GCC 12 enhanced -Waddress when comparing array address to null [0], > which warns: > > drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’: > drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must not be NULL [-Waddress] > 257 | if (vp_dev->msix_affinity_masks[i]) > | ^~~~~~ > > In fact, the verification is comparing the result of a pointer > arithmetic, the address "msix_affinity_masks + i", which will always > evaluate to true. > > Under the hood, free_cpumask_var() calls kfree(), which is safe to pass > NULL, not requiring non-null verification. So remove the verification > to make compiler happy (happy compiler, happy life). > > [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103 > > Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > --- > drivers/virtio/virtio_pci_common.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > index d724f676608b..5046efcffb4c 100644 > --- a/drivers/virtio/virtio_pci_common.c > +++ b/drivers/virtio/virtio_pci_common.c > @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev) > > if (vp_dev->msix_affinity_masks) { > for (i = 0; i < vp_dev->msix_vectors; i++) > - if (vp_dev->msix_affinity_masks[i]) > - free_cpumask_var(vp_dev->msix_affinity_masks[i]); > + free_cpumask_var(vp_dev->msix_affinity_masks[i]); > } > > if (vp_dev->msix_enabled) {
After I sent this message, I realized that Christophe (copied here) had already proposed a fix:
https://lore.kernel.org/lkml/20220414150855.2407137-4-dinechin@redhat.com/
Christophe,
Since free_cpumask_var() calls kfree() and kfree() is null-safe, can we just drop this null verification and call free_cpumask_var() right away?
-- Murilo
| |