From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40509) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akTrW-0007ZS-Gv for qemu-devel@nongnu.org; Mon, 28 Mar 2016 05:53:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1akTrS-00072d-H6 for qemu-devel@nongnu.org; Mon, 28 Mar 2016 05:53:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59095) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akTrS-00072Z-C2 for qemu-devel@nongnu.org; Mon, 28 Mar 2016 05:53:26 -0400 References: <1459149797-24108-1-git-send-email-marcel@redhat.com> <56F8EE96.2090709@weilnetz.de> From: Marcel Apfelbaum Message-ID: <56F8FF0D.5030503@redhat.com> Date: Mon, 28 Mar 2016 12:53:17 +0300 MIME-Version: 1.0 In-Reply-To: <56F8EE96.2090709@weilnetz.de> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] pci: fix compilation warnings List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, armbru@redhat.com, mst@redhat.com On 03/28/2016 11:43 AM, Stefan Weil wrote: > Am 28.03.2016 um 09:23 schrieb Marcel Apfelbaum: >> Fix 'error: shift exponent -1 is negative' warning >> by adding a corresponding assert. >> >> Reported-by: Peter Maydell >> Signed-off-by: Markus Armbruster >> Signed-off-by: Marcel Apfelbaum >> --- >> hw/pci/pci.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index e67664d..a1d41aa 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -163,11 +163,15 @@ int pci_bar(PCIDevice *d, int reg) >> >> static inline int pci_irq_state(PCIDevice *d, int irq_num) >> { >> + assert(irq_num >= 0); >> + >> return (d->irq_state >> irq_num) & 0x1; >> } >> >> static inline void pci_set_irq_state(PCIDevice *d, int irq_num, int level) >> { >> + assert(irq_num >= 0); >> + >> d->irq_state &= ~(0x1 << irq_num); >> d->irq_state |= level << irq_num; >> } > > Do we use negative values for irq_num anywhere? Hi Stefan, I didn't see any irq_nu assignments to a negative value but there are some cases where it can be negative due to arithmetical operations like: hw/pci-host/bonito.c:651: int internal_irq = irq_num - BONITO_IRQ_BASE; On other cases we look for negative value: hw/ppc/ppc4xx_devs.c:171: if (irq_num < 0 || irq_num > 3) or hw/ppc/ppc4xx_pci.c:263: if (irq_num < 0) > If not, using an unsigned irq_num might be a better solution. All of the above are manageable, of course, but we are close to hard freeze (tomorrow) and the scope of this patch is much smaller (fix a compilation warning) I think is definitely worth looking into it, but maybe as part of QEMU 2.7. Thanks, Marcel > > Stefan >