[ipxe-devel] [PATCH] fix compile error in isabus_probe with gcc4.7

[Jason O\'Brien]

Olaf Hering <olaf at ...> writes:

> 
> On Fri, Mar 16, Michael Brown wrote:
> 
> > On Friday 16 Mar 2012 13:43:10 Olaf Hering wrote:
> > > --- ipxe.orig/src/drivers/bus/isa.c
> > > +++ ipxe/src/drivers/bus/isa.c
> > > @@ -97,6 +97,9 @@ static int isabus_probe ( struct root_de
> > >  	int ioidx;
> > >  	int rc;
> > > 
> > > +	if ( ISA_EXTRA_PROBE_ADDR_COUNT == 0 )
> > > +		return 0;
> > > +
> > >  	for_each_table_entry ( driver, ISA_DRIVERS ) {
> > >  		for ( ioidx = ISA_IOIDX_MIN ( driver ) ;
> > >  		      ioidx <= ISA_IOIDX_MAX ( driver ) ; ioidx++ ) {
> > 
> > This change would cause isabus_probe() to fail to use the driver's own 
built-
> > in probe list unless some additional probe addresses were also specified via 
> > ISA_PROBE_ADDRS, which would be incorrect behaviour.
> > 
> > The intended behaviour is that if ISA_PROBE_ADDRS is not defined, then 
> > isabus_probe() should just use the driver's own probe list.  Any additional 
> > addresses specified in ISA_PROBE_ADDRS should be tried before those in the 
> > driver's own list.
> 
> How would you fix the code in case the array is empty because
> ISA_PROBE_ADDRS is not defined?
> 
> Olaf
> 

I looked through this and it seems to me it's a false positive, GCC can't 
properly track the branching based on negative/positive index. It's not the only 
one in the repo with GCC4.7 either.

Here's a harmless patch that lets GCC get around it in this corner case, but 
isn't worth committing:

diff -r ipxe.orig/src/drivers/bus/isa.c ipxe/src/drivers/bus/isa.c
37a38
> 
47a49
> #ifdef ISA_PROBE_ADDRS
51a54,57
> #else
> #define ISA_IOADDR( driver, ioidx )                                     \
>         (driver)->probe_addrs[(ioidx)]
> #endif

Bypasses the funny business, but would still throw an error if we DID pass 
ISA_PROBE_ADDRS. At least it won't break silently if upstream xen changes ;)

I'd give you a patch for the rest, but after fixing them all I checked IPXE 
upstream @ git.ipxe.org/ipxe.git and they seem to already have patches 
committed.