qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] pcie: simplify pcie_add_capability()


From: Peter Xu
Subject: Re: [Qemu-devel] [PATCH] pcie: simplify pcie_add_capability()
Date: Thu, 16 Feb 2017 10:23:27 +0800
User-agent: Mutt/1.5.24 (2015-08-30)

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 <address@hidden>
> >---
> > 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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]