Date: Fri, 01 Mar 2002 20:34:16 +0000 From: Mark Murray <mark@grondar.za> To: Bruce Evans <bde@zeta.org.au> Cc: arch@FreeBSD.ORG Subject: Re: Warning and lint(1) fixes. Review please. Message-ID: <200203012034.g21KYGg4074172@grimreaper.grondar.org> In-Reply-To: <20020302060943.U58081-100000@gamplex.bde.org> ; from Bruce Evans <bde@zeta.org.au> "Sat, 02 Mar 2002 06:36:13 %2B1100." References: <20020302060943.U58081-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
> > +#else
> > +#define ATOMIC_ASM(NAME, TYPE, OP, CONS, V)
> > +#endif
>
> Should be an extern function for the non-gcc case. We already have some
> support for this (we build extern versions in atomic.c).
Hmm. Nice. I like that.
After looking, it is nasty, because there is a type in there, so I've
made it into a #define that also declares a n extern function.
> > @@ -112,6 +116,7 @@
> > {
> > int res = exp;
> >
> > +#ifdef __GNUC__
> > __asm __volatile(
> > " pushfl ; "
> > " cli ; "
> > @@ -127,6 +132,7 @@
> > : "r" (src), /* 1 */
> > "m" (*(dst)) /* 2 */
> > : "memory");
> > +#endif
> >
> > return (res);
> > }
>
> This works (static function instead of extern), but it gives messier code.
> Keep the non-gcc declarations separate.
IMO, that creates divergent declarations, but other files do that too,
so NP.
> > Index: i386/include/bus_at386.h
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/i386/include/bus_at386.h,v
> > retrieving revision 1.18
> > diff -u -d -r1.18 bus_at386.h
> > --- i386/include/bus_at386.h 18 Feb 2002 13:43:19 -0000 1.18
> > +++ i386/include/bus_at386.h 24 Feb 2002 21:28:54 -0000
> > @@ -274,6 +274,7 @@
> > else
> > #endif
> > {
> > +#ifdef __GNUC__
> > __asm __volatile(" \n\
> > cld \n\
> > 1: movb (%2),%%al \n\
>
> As in atomic.h, but more so. There are zillions of interfaces in this file.
> I'm surprised that you didn't have to change more.
:-) Me too. I'll fix.
> > @@ -374,7 +380,8 @@
> > if (tag == I386_BUS_SPACE_IO)
> > #endif
> > {
> > - int _port_ = bsh + offset; \
> > + int _port_ = bsh + offset;
>
> OK to fix all of these :-).
Cool!
> > Index: i386/include/pcpu.h
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/i386/include/pcpu.h,v
> > retrieving revision 1.32
> > diff -u -d -r1.32 pcpu.h
> > --- i386/include/pcpu.h 11 Dec 2001 23:33:40 -0000 1.32
> > +++ i386/include/pcpu.h 28 Feb 2002 10:44:43 -0000
> > @@ -32,8 +32,22 @@
> > #ifdef _KERNEL
> >
> > #ifndef __GNUC__
> > -#error gcc is required to use this file
> > -#endif
> > +
> > +#ifndef lint
> > +#error gcc or lint is required to use this file
> > +#else /* lint */
> > +#define __PCPU_PTR(name)
> > +#define __PCPU_GET(name)
> > +#define __PCPU_SET(name, val)
>
> I can't think of any good way to handle this.
OK for the above as a stopgap?
> > Index: sys/cdefs.h
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/sys/cdefs.h,v
> > retrieving revision 1.49
> > diff -u -d -r1.49 cdefs.h
> > --- sys/cdefs.h 4 Dec 2001 01:29:54 -0000 1.49
> > +++ sys/cdefs.h 19 Feb 2002 15:32:10 -0000
> > @@ -112,6 +112,7 @@
> > * properly (old versions of gcc-2 supported the dead and pure features
> > * in a different (wrong) way).
> > */
> > +#ifdef __GNUC__
> > #if __GNUC__ < 2 || __GNUC__ == 2 && __GNUC_MINOR__ < 5
> > #define __dead2
> > #define __pure2
>
> Bogus. If __GNUC__ is not defined, then it is less than 2.
>
> > @@ -176,7 +177,6 @@
> > #define __printf0like(fmtarg, firstvararg)
> > #endif
> >
> > -#ifdef __GNUC__
> > #define __strong_reference(sym,aliassym) \
> > extern __typeof (sym) aliassym __attribute__ ((__alias__ (#sym)));
> > #ifdef __ELF__
>
> This gcc ifdef is unrelated to the one above. It should have its own
> version checks. I think __alias__ is a syntax error except for relative
> recent versions of gcc.
The above is a move of the "#ifdef __GNUC__" to a few lines above where it
was to enclose more GNU C specific code.
> > @@ -244,7 +244,7 @@
> > #if !defined(lint) && !defined(STRIP_FBSDID)
> > #define __FBSDID(s) __IDSTRING(__CONCAT(__rcsid_,__LINE__),s)
> > #else
> > -#define __FBSDID(s) struct __hack
> > +#define __FBSDID(s)
> > #endif
> > #endif
>
> This breaks enforcement of a semicolon after __FBSDID().
But it (and friends) cause multiple whinings about multiple declarations
of 'struct __hack'. I can work around that, though.
> > Index: sys/eventhandler.h
> > ===================================================================
> > RCS file: /home/ncvs/src/sys/sys/eventhandler.h,v
> > retrieving revision 1.17
> > diff -u -d -r1.17 eventhandler.h
> > --- sys/eventhandler.h 12 Sep 2001 08:38:05 -0000 1.17
> > +++ sys/eventhandler.h 19 Feb 2002 22:17:52 -0000
> > @@ -75,31 +75,33 @@
> > struct eventhandler_entry ee; \
> > type eh_func; \
> > }; \
> > -struct __hack
> > +struct __hack_ ## name
>
> The same incomplete struct should be used for this everywhere. rm-rf any
> lint that doesn't like this.
Point taken :-). Fixed.
> > +#define EVENTHANDLER_FAST_INVOKE(name, args...) \
> > +do { \
> > + struct eventhandler_list *_el = &Xeventhandler_list_ ## name ; \
> > + struct eventhandler_entry *_ep, *_en; \
> > + \
> > + if (_el->el_flags & EHE_INITTED) { \
> > + lockmgr(&_el->el_lock, LK_EXCLUSIVE, NULL, curthread); \
> > + _ep = TAILQ_FIRST(&(_el->el_entries)); \
> > + while (_ep != NULL) { \
> > + _en = TAILQ_NEXT(_ep, ee_link); \
> > + ((struct eventhandler_entry_ ## name *)_ep)->eh_func( \
> > + _ep->ee_arg , \
> > + ## args \
> > + ); \
> > + _ep = _en; \
> > + } \
> > + lockmgr(&_el->el_lock, LK_RELEASE, NULL, curthread); \
> > + } \
> > } while (0)
>
> This is almost readable now. It still has 4-char indents.
I've fixed that file using tabs (+ 4-space continuation indents).
So OK so far? I'll post again a commit candidate before I commit this.
M
--
o Mark Murray
\_
O.\_ Warning: this .sig is umop ap!sdn
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200203012034.g21KYGg4074172>
