From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LXgI2-0005gN-Qr for qemu-devel@nongnu.org; Thu, 12 Feb 2009 13:27:58 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LXgI0-0005fa-O6 for qemu-devel@nongnu.org; Thu, 12 Feb 2009 13:27:57 -0500 Received: from [199.232.76.173] (port=56776 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LXgI0-0005fW-KG for qemu-devel@nongnu.org; Thu, 12 Feb 2009 13:27:56 -0500 Received: from mx2.redhat.com ([66.187.237.31]:56810) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LXgI0-0000Xl-6u for qemu-devel@nongnu.org; Thu, 12 Feb 2009 13:27:56 -0500 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n1CIRtMW015640 for ; Thu, 12 Feb 2009 13:27:55 -0500 Date: Thu, 12 Feb 2009 16:27:26 -0200 From: Marcelo Tosatti Subject: Re: [Qemu-devel] [6609] Parse full PCI device addresses (Markus Armbruster) Message-ID: <20090212182726.GA19553@amt.cnet> References: <200902121539.26892.paul@codesourcery.com> <20090212155605.GA18654@amt.cnet> <200902121618.17411.paul@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200902121618.17411.paul@codesourcery.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Markus Armbruster On Thu, Feb 12, 2009 at 04:18:16PM +0000, Paul Brook wrote: > > > If we're rejecting nonzero domains then having the parse routine return a > > > domain value seems wrong. It's just going to make it harder to verify > > > correct operation when domains are implemented. > > > > + /* Note: QEMU doesn't implement domains other than 0 */ > > + if (dom != 0 || pci_find_bus(bus) == NULL) { > > + fprintf(stderr, "PCI device address %s not supported", addr); > > + return -1; > > + } > > > > + if (!strcmp(devaddr, "auto")) { > > + *domp = *busp = 0; > > + *slotp = -1; > > + /* want to support dom/bus auto-assign at some point */ > > + return 0; > > + } > > > > We return domain 0. I considered domain 0 as implicit at the moment, is > > that wrong? > > > > I can't see where you're getting at. > > You're returning a value that's always known to be zero. IMHO That's worse > than not returning a value at all. > > This implies users of this function will either ignore the value or have > redundant, untested (i.e. probably bitrotten) code to handle nonzero domains. > Either way, it's liable the break horribly at runtime as soon as we start > trying to support multiple domains. > > If pci_parse_devaddr is defined to only handle zero domains, then it should > not be returning a domain value. If/when we implement multiple PCI domains we > can change the interface, and get nice compiler errors in all the other code > we forgot to update. That makes sense. > Alternatively, have the parse routine return the full tuple, and enforce > domain == 0 elsewhere. OK, i'll submit something later in the week so you can ACK/NACK. We want to pass all the way down to pci_register_device, and kill the "-1" parameter that drivers use today (so one can statically assign bus/slot and eventually domain). But OK, will drop domain from the API for now.