[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [6609] Parse full PCI device addresses (Markus Armbrust
From: |
Paul Brook |
Subject: |
Re: [Qemu-devel] [6609] Parse full PCI device addresses (Markus Armbruster) |
Date: |
Thu, 12 Feb 2009 16:18:16 +0000 |
User-agent: |
KMail/1.9.9 |
> > 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.
Alternatively, have the parse routine return the full tuple, and enforce
domain == 0 elsewhere.
Paul