aic7xxx funcs without return values

aic7xxx funcs without return values

[J . A . Magallon]

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:jamagallon@able.es                                          $> more beer

Linux werewolf 2.4.2-ac13 #3 SMP Wed Mar 7 00:09:17 CET 2001 i686

Re: aic7xxx funcs without return values

[Justin T. Gibbs]

>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

Re: aic7xxx funcs without return values

[Alan Cox]

> 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.

Re: aic7xxx funcs without return values

[Justin T. Gibbs]

>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