Jean-Philippe Brucker <jean-philippe.brucker@xxxxxxx> writes:
> On 01/08/17 17:04, Punit Agrawal wrote:
> [...]
>> I think I've figured out what's going wrong. pci_for_each_cap is defined
>> as -
>>
>> #define pci_for_each_cap(pos, cap, hdr) \
>> for ((pos) = (hdr)->capabilities & ~3; \
>> (cap) = (void *)(hdr) + (pos), (pos) != 0; \
>> (pos) = ((struct pci_cap_hdr *)(cap))->next & ~3)
>>
>> Here cap is being assigned before the pos != 0 check. So in the last
>> iteration through the loop, cap will point to the start of the PCI
>> header - which is then used to set "last->next = pos".
>
> Ah right, sorry about that. How about moving the pos check in front? Does
> the following work for you?
>
> diff --git a/include/kvm/pci.h b/include/kvm/pci.h
> index c5fc8254..37a65956 100644
> --- a/include/kvm/pci.h
> +++ b/include/kvm/pci.h
> @@ -142,9 +142,9 @@ struct pci_device_header {
> enum irq_type irq_type;
> };
>
> -#define pci_for_each_cap(pos, cap, hdr) \
> - for ((pos) = (hdr)->capabilities & ~3; \
> - (cap) = (void *)(hdr) + (pos), (pos) != 0; \
> +#define pci_for_each_cap(pos, cap, hdr) \
> + for ((pos) = (hdr)->capabilities & ~3; \
> + (pos) != 0 && ({ (cap) = (void *)(hdr) + (pos); 1; }); \
> (pos) = ((struct pci_cap_hdr *)(cap))->next & ~3)
>
> int pci__init(struct kvm *kvm);
The compiler throws a warning with the above change and fails to compile
(as all warnings are treated as error by kvmtool build) -
vfio/pci.c: In function ‘vfio_pci_setup_device’:
vfio/pci.c:511:14: error: ‘last’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
last->next = pos;
~~~~~~~~~~~^~~~~
vfio/pci.c:507:23: note: ‘last’ was declared here
struct pci_cap_hdr *last;
^~~~
