Date: Sat, 2 Mar 2002 06:36:13 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Mark Murray <mark@grondar.za> Cc: <arch@FreeBSD.ORG> Subject: Re: Warning and lint(1) fixes. Review please. Message-ID: <20020302060943.U58081-100000@gamplex.bde.org> In-Reply-To: <200202281836.g1SIaog4051908@grimreaper.grondar.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 28 Feb 2002, Mark Murray wrote:
> Please review the enclosed fixes. I've been running most of them
> for more than a month, and they are heavily useful in fixing up
> lint moanings.
> Index: i386/include/atomic.h
> ===================================================================
> RCS file: /home/ncvs/src/sys/i386/include/atomic.h,v
> retrieving revision 1.26
> diff -u -d -r1.26 atomic.h
> --- i386/include/atomic.h 28 Feb 2002 06:17:05 -0000 1.26
> +++ i386/include/atomic.h 28 Feb 2002 09:43:25 -0000
> @@ -89,6 +89,7 @@
> * The assembly is volatilized to demark potential before-and-after side
> * effects if an interrupt or SMP collision were to occur.
> */
> +#ifdef __GNUC__
> #define ATOMIC_ASM(NAME, TYPE, OP, CONS, V) \
> static __inline void \
> atomic_##NAME##_##TYPE(volatile u_##TYPE *p, u_##TYPE v)\
> @@ -97,6 +98,9 @@
> : "+m" (*p) \
> : CONS (V)); \
> }
> +#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).
> @@ -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.
> 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.
> @@ -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 :-).
> 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.
> 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.
> @@ -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().
> 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.
> +#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.
Bruce
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?20020302060943.U58081-100000>