On Wed, Feb 15, 2017 at 04:25:05PM +0200, Marcel Apfelbaum wrote:
> On 02/14/2017 09:51 AM, Peter Xu wrote:
> >When we add PCIe extended capabilities, we should be following the rule
> >that we add the head extended cap (at offset 0x100) first, then the rest
> >of them. Meanwhile, we are always adding new capability bits at the end
> >of the list. Here the "next" looks meaningless in all cases since it
> >should always be zero (along with the "header").
> >
> >Simplify the function a bit, and it looks more readable now.
> >
> >Signed-off-by: Peter Xu <pet...@redhat.com>
> >---
> > hw/pci/pcie.c | 15 ++++-----------
> > 1 file changed, 4 insertions(+), 11 deletions(-)
> >
> >diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> >index cbd4bb4..e0e6f6a 100644
> >--- a/hw/pci/pcie.c
> >+++ b/hw/pci/pcie.c
> >@@ -664,30 +664,23 @@ void pcie_add_capability(PCIDevice *dev,
> > uint16_t cap_id, uint8_t cap_ver,
> > uint16_t offset, uint16_t size)
> > {
> >- uint32_t header;
> >- uint16_t next;
> >-
> > assert(offset >= PCI_CONFIG_SPACE_SIZE);
> > assert(offset < offset + size);
> > assert(offset + size <= PCIE_CONFIG_SPACE_SIZE);
> > assert(size >= 8);
> > assert(pci_is_express(dev));
> >
> >- if (offset == PCI_CONFIG_SPACE_SIZE) {
> >- header = pci_get_long(dev->config + offset);
> >- next = PCI_EXT_CAP_NEXT(header);
> >- } else {
> >+ if (offset != PCI_CONFIG_SPACE_SIZE) {
> > uint16_t prev;
> >
> > /* 0 is reserved cap id. use internally to find the last capability
> > in the linked list */
> >- next = pcie_find_capability_list(dev, 0, &prev);
> >-
> >+ assert(pcie_find_capability_list(dev, 0, &prev) == 0);
>
> Hi Peter,
>
> It is not recommended to use assert with an expression with side-effects.
Exactly. Thanks Marcel, I'll repost.
-- peterx
