Patch for review (was: Re: Proposal to revise the new parsing of PCI selectors (was: Re: HEADS UP: [cvs commit: src UPDATING src/share/man/man4 pci.4?src/share/man/man9 pci.9?src/sys/amd64/include?legacyvar.h?src/sys/amd64/amd64 legacy.c?src/sys/amd64/pci pci_bus.c?src/sys/arm/xscale/i80321?i80321_pci.c src/sys/arm/xscale/ixp425 ...))

On 2007-10-01 14:20 -0400, John Baldwin <jhb_at_freebsd.org> wrote:
> On Monday 01 October 2007 09:25:48 am Marius Strobl wrote:
> > On Mon, Oct 01, 2007 at 02:49:12PM +0200, Stefan Esser wrote:
> > > Well, since it was me who chose to parse it that way, when pciconf
> > > saw the light of day, I can say that the logical extension appears
> > > to be the support of 3 formats for the PCI device selector:
> > > 
> > > pci1:2:3:4  (full, domain/bus/slot/function, required if domain!=0)
> > > pci2:3:4    (abridged, in case the domain is "0")
> > > pci2:3      (abridged, in case the domain and function are "0")
> > 
> > I'm ok with what you propose, I'd wait for John to comment
> > whether he sees any issues regarding the hints feature he is
> > working on though.
> 
> This sounds good to me.

Ok, I've tested the following patch, which also restores a feature
of the original code, when it was not clear, whether the separator
character was supposed to be ":" or "." (i.e., the new version does
accept both ":" and "." as separator). This would allow to use the 
same selectors (with ".") in pciconf and the hints file ...

I'd of course be willing to commit both changes separately (first 
the parsing of selectors with 2, 3 or 4 elements, then equivalence
of ":" and "." as separators).

The code wrapped in "#if 0" is not to be committed, I've included
it just in case anybody wants to perform some tests and to check 
the parsing results.

Regards, STefan



Index: usr.sbin/pciconf/pciconf.c
===================================================================
RCS file: /usr/cvs/src/usr.sbin/pciconf/pciconf.c,v
retrieving revision 1.28
diff -u -3 -r1.28 pciconf.c
--- usr.sbin/pciconf/pciconf.c	30 Sep 2007 11:05:17 -0000	1.28
+++ usr.sbin/pciconf/pciconf.c	3 Oct 2007 10:33:03 -0000
_at__at_ -486,6 +486,8 _at__at_
 	char *ep = strchr(str, '_at_');
 	char *epbase;
 	struct pcisel sel;
+	u_int8_t selarr[4];
+	int i;
 
 	if (ep == NULL)
 		ep = (char *)str;
_at__at_ -496,22 +498,25 _at__at_
 
 	if (strncmp(ep, "pci", 3) == 0) {
 		ep += 3;
-		sel.pc_domain = strtoul(ep, &ep, 0);
-		if (!ep || *ep++ != ':')
-			errx(1, "cannot parse selector %s", str);
-		sel.pc_bus = strtoul(ep, &ep, 0);
-		if (!ep || *ep++ != ':')
-			errx(1, "cannot parse selector %s", str);
-		sel.pc_dev = strtoul(ep, &ep, 0);
-		if (!ep || *ep != ':') {
-			sel.pc_func = 0;
-		} else {
-			ep++;
-			sel.pc_func = strtoul(ep, &ep, 0);
-		}
-		if (*ep == ':')
-			ep++;
+		i = 0;
+		do {
+			selarr[i++] = strtoul(ep, &ep, 0);
+		} while ((*ep == ':' || *ep == '.') && *++ep != '\0' && i < 4);
+
+		if (i > 2)
+			sel.pc_func = selarr[--i];
+		else
+			sel.pc_func = 0; 
+		sel.pc_dev = selarr[--i];
+		sel.pc_bus = selarr[--i];
+		if (i > 0)
+			sel.pc_domain = selarr[--i];
+		else
+			sel.pc_domain = 0;
 	}
+#if 0
+printf ("SELECTOR=(%d:%d:%d:%d) *ep=0x%02x\n", sel.pc_domain, sel.pc_bus, sel.pc_dev, sel.pc_func, *ep);
+#endif
 	if (*ep != '\x0' || ep == epbase)
 		errx(1, "cannot parse selector %s", str);
 	return sel;