i386 compile sys/dev/ie

[Bruce Evans]

On Wed, 28 Dec 2011, Bruce Evans wrote:

> On Wed, 28 Dec 2011, Sergey Kandaurov wrote:

>> These were used in probe routine and are left from the newbus rewrite.
>> I hacked ie a bit to build cleanly. [Not sure if I did this correctly.]
>
> Use of the __DEVOLATILE() abomination is never correct.  It exploits the
> bug that -Wcast-qual is broken for casts through [u]intptr_t.

PS: I used to maintain some bad fixes in this area for ie, but now
have only the following:

% .Index: if_ie.c
% .===================================================================
% .RCS file: /home/ncvs/src/sys/dev/ie/if_ie.c,v
% .retrieving revision 1.99
% .diff -u -2 -r1.99 if_ie.c
% .--- if_ie.c	17 Mar 2004 17:50:35 -0000	1.99
% .+++ if_ie.c	31 May 2004 06:57:05 -0000
% .@@ -159,4 +159,7 @@
% . #define IE_BUF_LEN	ETHER_MAX_LEN	/* length of transmit buffer */
% . 
% .+/* XXX this driver uses `volatile' and `caddr_t' to a fault. */
% .+typedef	volatile char *v_caddr_t;	/* core address, pointer to volatile */
% .+
% . /* Forward declaration */
% . struct ie_softc;

I never allowed the __DEFOO() abominations in my <sys/cdefs.h>, and at
one time had all uses to them in /usr/src removed and ready to commit
(there were about 2 of them.  There are hundreds if not thousands now :-().
c_caddr_t and v_caddr_t are as abominable as __DEFOO(), so of course I
never allowed them in my <sys/types.h>.  But v_caddr_t is still easy to
remove, since it is only used in the ie driver.  It is used the break
warnings from -Wcast-qual that more natural casts would give (but warnings
still result from the implicit conversion for bcopy()):

% if_ie.c:static v_caddr_t setup_rfa		(struct ie_softc *, v_caddr_t);

The very idea of a v_caddr_t is nonsense.  caddr_t is a bad old type for
a "core address".  In most cases, it means the same as "void *", but was
used because "void *" didn't exist.  In FreeBSD, it must be precisely
"char *" to work, since pointer arithmetic is perpetrated on it (the
pointer arithmetic defeats its opaqueness).  In a few cases, it is
used for physical or device memory accesses.  In must places, it should
be spelled "void *", and in other places it should be replaced by
a vm virtual or physical address type, or a bus space type.

To this mess, the ie driver, but mercifully no other code in /usr/src,
adds v_caddr_t.  caddr_t is supposed to be opaque, but if we just cast
to that we will get a cast-qual error if we start with one of ie's
excessively qualified pointers.  Also, "const caddr_t" doesn't work
since it puts the const in the wrong place.  This is "solved" by looking
inside caddr_t to add the const there.

ie uses v_caddr_t as follows:

First, in the above, some of the excessive qualifiers are in setup_rfa()'s
return type and second parameter...

% if_ie.c:	setup_rfa(sc, (v_caddr_t) sc->rframes[0]);
% if_ie.c:	setup_rfa(sc, (v_caddr_t) sc->rframes[0]);	/* ignore cast-qual */

... this makes even calling setup_rfa() difficult.  We start with a
volatile struct ie_mumble **rframes.  We can't just cast this to void *
or caddr_t since this would cause a -Wcast-qual warning.  Casting it
to "volatile void *" would be even worse, since it removes a volatile
qualifier that is in the (technically) right place and adds it back in
a wrong place.  So we use v_caddr_t to keep it in the same place.

% if_ie.c:			bcopy((v_caddr_t) (sc->cbuffs[head] + offset),
% if_ie.c:			bcopy((v_caddr_t) (sc->cbuffs[head] + offset),
% if_ie.c:		bcopy((v_caddr_t) (sc->cbuffs[head] + offset),

Similarly.  cbuffs is a volatile u_char **.  We want to aplply bcopy()
to (cbuffs[head] + offset), which is a volatile u_char *.  We assume that
bcopy() is still the 1980's version which takes a caddr_t, although it
took a (void *) even in 1992.  So we originally tried to bogusly cast the
arg to caddr_t.  This had no effect, as would the more correct cast to
(void *) have done, since the prototype converts to void * anyway.
Eventually, -Wcast-qual was used and this bug was detected.  (There may be
a more serious bug involving char * being incompatble with void *.  A
grandfather kludge keeps char * equivalent to void * in many context.
But it is not clear what happens with complicated qualifiers.)  Some
time later, the -Wcast-qual was broken using a volatile cast.  But
the warning for converting away the qualifier by the prototype
remained.  Omitting the cast would have had the same effect.

% if_ie.c:	bcopy((v_caddr_t) (sc->rframes[num]), &rfd,

Similarly.

% if_ie.c:static v_caddr_t
% if_ie.c:setup_rfa(struct ie_softc *sc, v_caddr_t ptr)

The definition matching the above prototype.

% if_ie.c:	bcopy((v_caddr_t) sc->mcast_addrs, (v_caddr_t) cmd->ie_mcast_addrs,

Similarly.

% if_ie.c:		bzero((v_caddr_t) sc->xmit_cmds[i], sizeof *sc->xmit_cmds[i]);
% if_ie.c:		bzero((v_caddr_t) sc->xmit_buffs[i], sizeof *sc->xmit_buffs[i]);

Same mistakes for bzero().

My change isolates the v_caddr_t mistake in ie.

The c_caddr_t mistake is used 17 times in the kernel and 75 times in /usr/src.

Bruce