Skip site navigation (1)Skip section navigation (2)
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>