cvs commit: src/sys/boot/arc/include arcfuncs.h src/sys/boot/i386/boot2 boot2.c src/sys/dev/aic7xxx/aicasm aicasm.c src/sys/dev/cx machdep.h src/sys/dev/ichsmb ichsmb.c src/sys/dev/iir iir.h src/sys/dev/isp ispvar.h src/sys/dev/mcd mcdreg.h ...

[Maxime Henrion]

Ruslan Ermilov wrote:
> On Fri, Mar 12, 2004 at 01:45:45PM -0800, Tom Rhodes wrote:
> [...]
> > Index: src/sys/dev/mcd/mcdreg.h
> >  
> > -#ifdef __GNUC__
> > -#if __GNUC__ >= 2
> > +#if defined(__GNUC__) || defined(__INTEL_COMPILER)
> > +#if __GNUC__ >= 2 || defined(__INTEL_COMPILER)
> >  #pragma pack(1)
> >  #endif
> >  #endif
> >  
> > -#ifdef __GNUC__
> > -#if __GNUC__ >= 2
> > +#if defined(__GNUC__) || defined(__INTEL_COMPILER)
> > +#if __GNUC__ >= 2 || defined(__INTEL_COMPILER)
> >  #pragma pack(4)
> >  #endif
> >  #endif
> > 
> These ifdefs are broken.
> 
> #if (defined(__GNUC__) && __GNUC >= 2) || defined(__INTEL_COMPILER)
> 
> would be more correct.
> 
> > Index: src/sys/dev/scd/scdreg.h
> > -#ifdef __GNUC__
> > -#if __GNUC__ >= 2
> > +#if defined(__GNUC__) || defined(__INTEL_COMPILER)
> > +#if __GNUC__ >= 2 || defined(__INTEL_COMPILER)
> >  #pragma pack(1)
> >  #endif
> >  #endif
> > 
> Ditto.
> 
> > Index: src/sys/i386/i386/in_cksum.c
> [...]
> > +#if !defined(__GNUC__) || defined(__INTEL_COMPILER)
> > 
> What, __INTEL_COMPILER can have __GNUC__ defined?
> 
> > Index: src/sys/i386/include/_types.h
> > diff -u src/sys/i386/include/_types.h:1.7 src/sys/i386/include/_types.h:1.8
> > --- src/sys/i386/include/_types.h:1.7	Sat Mar 29 21:24:52 2003
> > +++ src/sys/i386/include/_types.h	Fri Mar 12 13:45:29 2004
> > @@ -113,12 +113,12 @@
> >  /*
> >   * Unusual type definitions.
> >   */
> > -#if defined(__GNUC__) && (__GNUC__ == 2 && __GNUC_MINOR__ > 95 || __GNUC__ >= 3)
> > +#if (defined(__GNUC__) && (__GNUC__ == 2 && __GNUC_MINOR__ > 95 || __GNUC__ >= 3) && !defined(__INTEL_COMPILER))
> > 
> Redundant surrounding parenthesis.
> 
> > Index: src/sys/i386/include/in_cksum.h
> [...]
> > +#if defined(__GNUC__) && !defined(__INTEL_COMPILER)
> > 
> I think these are mutually exclusive already, no?
> 
> > Index: src/sys/i386/include/stdarg.h
> [...]
> > -#if defined(__GNUC__) && (__GNUC__ == 2 && __GNUC_MINOR__ > 95 || __GNUC__ >= 3)
> > +#if (defined(__GNUC__) && (__GNUC__ == 2 && __GNUC_MINOR__ > 95 || __GNUC__ >= 3) && !defined(__INTEL_COMPILER))
> >  
> Redundant parenthesis.
> 
> > Index: src/sys/sys/cdefs.h
> > -#ifndef __GNUC__
> > +#if !(defined(__GNUC__) || defined(__INTEL_COMPILER))
> > 
> Mechanical non-optimized changes?  ;)
> 
> > -#if !__GNUC_PREREQ__(2, 5)
> > +#if !__GNUC_PREREQ__(2, 5) && !defined(__INTEL_COMPILER)
> > 
> Better than above.  ;)

It would be even better IMHO if you had used __GNUC_PREREQ__ in other
files, to simplify all those tests.  There is however one small nit with
__GNUC_PREREQ__, that bde@ pointed to me in that in FreeBSD, we usually
prefer calling this kind of macros __GNUC_PREREQ, ie without the
underscores at the end.  I left them in for now because I wanted to keep
compatibility with OpenBSD (and NetBSD too IIRC), from which it was
taken.  If most people feel we should remove those, I'm not opposed to
it either (this _doesn't_ mean "let's go for another bikshed", thanks).

Cheers,
Maxime