Hi,
Just a note to make gcc 2.96 (and future) happy. The aic7xxx driver is full of
inline funcs that should return a value and do not do that:
gcc -D__KERNEL__ -I/usr/src/linux-2.4.2-ac14/include -Wall -Wstrict-prototypes
-O2 -fomit-frame-pointer -fno-strict-aliasing -pipe -mpreferred-stack-boundary=2
-march=i686 -c -o aic7xxx_linux.o aic7xxx_linux.c
In file included from aic7xxx_linux.c:131:
aic7xxx_osm.h: In function `ahc_pci_read_config':
aic7xxx_osm.h:839: warning: control reaches end of non-void function
That function, for example, is missing the returns and the breaks:
static __inline uint32_t
ahc_pci_read_config(ahc_dev_softc_t pci, int reg, int width)
{
switch (width) {
case 1:
{
uint8_t retval;
pci_read_config_byte(pci, reg, &retval);
return (retval);
}
case 2:
{
uint16_t retval;
pci_read_config_word(pci, reg, &retval);
return (retval);
}
case 4:
{
uint32_t retval;
pci_read_config_dword(pci, reg, &retval);
return (retval);
}
default:
panic("ahc_pci_read_config: Read size too big");
/* NOTREACHED */
}
}
Or if you are so worried about early returns and fast paths:
static __inline uint32_t
ahc_pci_read_config(ahc_dev_softc_t pci, int reg, int width)
{
uint8_t b_val;
uint16_t s_val;
uint32_t l_val;
switch (width) {
case 1:
pci_read_config_byte(pci, reg, &b_val);
return b_bal;
break;
case 2:
pci_read_config_word(pci, reg, &s_val);
return s_bal;
break;
case 4:
pci_read_config_dword(pci, reg, &l_val);
return l_bal;
break;
}
panic("ahc_pci_read_config: Read size too big");
return 0;
}
Stack is changed only once, gcc is happy, and perhaps the explicit breaks
help gcc to optimize the code.
--
J.A. Magallon $> cd pub
mailto:[email protected] $> more beer
Linux werewolf 2.4.2-ac13 #3 SMP Wed Mar 7 00:09:17 CET 2001 i686
>Hi,
>
>Just a note to make gcc 2.96 (and future) happy. The aic7xxx driver is full of
>inline funcs that should return a value and do not do that:
They don't return a value because doing so is meaningless. You aren't
going to get past the panic. The compiler should know that assuming
panic is properly tagged as a function that cannot return.
You may also want to check up on your C since having a break after
a return is, well, kinda silly. In all the usage of this inline, the
width is constant, so gcc should completely optimize away the switch
and surrounding code.
--
Justin
> They don't return a value because doing so is meaningless. You aren't
> going to get past the panic. The compiler should know that assuming
> panic is properly tagged as a function that cannot return.
>
> You may also want to check up on your C since having a break after
> a return is, well, kinda silly. In all the usage of this inline, the
> width is constant, so gcc should completely optimize away the switch
> and surrounding code.
Yep.
The bigger problem with that driver for pedants is that it contains globals with
names like 'hard_error' which are asking for clashes . Bizarrely all
the static functions are carefully named ahc_* and the globals are called
things like 'restart_squencer'
The EISA probe code is also horrible, but thats not Justin's fault. Someone
needs to put some eisa_* bus functions into the kernel for that and some
of the other drivers.
>The bigger problem with that driver for pedants is that it contains globals
>with names like 'hard_error' which are asking for clashes . Bizarrely all
>the static functions are carefully named ahc_* and the globals are called
>things like 'restart_squencer'
Such is the evolutionary nature of most drivers. I just spent a bit
of time cleaning this up. Some of the exported symbols didn't even
need to be in the current layout of the driver.
Thanks for the reminder.
Justin