cvs commit: src/sys/boot/arc/include arcfuncs.hsrc/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/s

[Bruce Evans]

On Sat, 13 Mar 2004, Tony Finch wrote:

> On Sat, 13 Mar 2004, Ruslan Ermilov wrote:
> > On Fri, Mar 12, 2004 at 01:45:45PM -0800, Tom Rhodes wrote:
> > >
> > > -#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.
>
> Why, given that an undefined macro is equivalent to 0 in this context?

I think there are only style bugs.  The outer ifdefs have no effect in
either the old version or the new version.

Also, the ifdefs shouldn't be needed, and using "#pragma pack" is
mainly a style bug.  This pragma is used in only 2 headers in /sys
(mcdreg.h and scdreg.h).  We use __packed almost everywhere else.  The
definition of __packed is centralized so that only 1 ifdef is needed
for it.  A few broken places hard-code the gccism "__attribute__(())".
This unportability is especially unneeded in mcdreg.h and scdreg.h.
All structs in these files are wrapped with "#pragma pack(1)" and
#pragma pack(4)" (except scdreg.h is missing the second pragma, so the
first pragma leaks out of it).  But these structs need packing less
than most, since they consist mainly of char fields.  Normal packing
gives the same results as "#pragma pack(1)" for all reasonable
compilers/machines, except possibly for some problematic bit-fields.

In mcdreg.h: the main problems are here:

% struct mcd_qchninfo {
% 	u_char  addr_type:4;
% 	u_char  control:4;
% 	u_char	trk_no;
% 	u_char	idx_no;
% 	bcd_t	trk_size_msf[3];
% 	u_char	:8;
% 	bcd_t	hd_pos_msf[3];
% };

(bcd_t is u_char.)  The u_char bit-fields can't be compiled by C compilers.
If they were replaced by u_int and "#pragma pack(1)" were removed, then gcc
would pack this struct internally (to 10 bytes) but pad it externally to
have size a multiple of sizeof(u_int).  Misdeclaring the bit-fields as
u_char prevents the external padding.  "#pragma pack(1)" seems to have no
additional affect.  It would just prevent the extern padding if the struct
were declared in C.

scdreg.h is similar except it has many more unportable bit-fields.

Bruce