On Mon, 13 Sep 2021 16:44:09 +0100
Alexandru Elisei <alexandru.elisei@xxxxxxx> wrote:
> Evaluate the "pci_hdr" argument before attempting to deference a field.
> This fixes cryptic errors like this one, which came about during a
> debugging session:
>
> vfio/pci.c: In function 'vfio_pci_bar_activate':
> include/kvm/pci.h:18:40: error: invalid type argument of '->' (have 'struct pci_device_header')
> pr_warning("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
> ^~
> vfio/pci.c:482:3: note: in expansion of macro 'pci_dev_warn'
> pci_dev_warn(&vdev->pci.hdr, "%s: BAR4\n", __func__);
>
> This is caused by the operator precedence rules in C, where pointer
> deference via "->" has a higher precedence than taking the address with the
> ampersand symbol. When the macro is substituted, it becomes
> &vdev->pci.hdr->vendor_id and it dereferences vdev->pci.hdr, which is not a
> pointer, instead of dereferencing &vdev->pci.hdr, which is a pointer, and
> quite likely what the author intended.
Indeed! Actually that should not need that many words, parameters in macros
should always be put in parentheses.
> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx>
Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
Cheers,
Andre
> ---
> include/kvm/pci.h | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/kvm/pci.h b/include/kvm/pci.h
> index 0f2d5bb..d6eb398 100644
> --- a/include/kvm/pci.h
> +++ b/include/kvm/pci.h
> @@ -13,15 +13,15 @@
> #include "kvm/kvm-arch.h"
>
> #define pci_dev_err(pci_hdr, fmt, ...) \
> - pr_err("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
> + pr_err("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
> #define pci_dev_warn(pci_hdr, fmt, ...) \
> - pr_warning("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
> + pr_warning("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
> #define pci_dev_info(pci_hdr, fmt, ...) \
> - pr_info("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
> + pr_info("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
> #define pci_dev_dbg(pci_hdr, fmt, ...) \
> - pr_debug("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
> + pr_debug("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
> #define pci_dev_die(pci_hdr, fmt, ...) \
> - die("[%04x:%04x] " fmt, pci_hdr->vendor_id, pci_hdr->device_id, ##__VA_ARGS__)
> + die("[%04x:%04x] " fmt, (pci_hdr)->vendor_id, (pci_hdr)->device_id, ##__VA_ARGS__)
>
> /*
> * PCI Configuration Mechanism #1 I/O ports. See Section 3.7.4.1.
