Re: [PATCH] X25: Add ITU-T facilites

[Andrew Hendry <ahendry@tusc.com.au>]

On Wed, 2005-10-19 at 01:48, linux-os (Dick Johnson) wrote:
> On Tue, 18 Oct 2005, Arnaldo Carvalho de Melo wrote:
> 
> > Em Tue, Oct 18, 2005 at 03:23:18PM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ escreveu:
> >> In article <1129615767.3695.15.camel@localhost.localdomain> (at Tue, 18 Oct 2005 16:09:27 +1000), Andrew Hendry <ahendry@tusc.com.au> says:
> >>
> >>> +/*
> >>> +*     ITU DTE facilities
> >>> +*     Only the called and calling address
> >>> +*     extension are currently implemented.
> >>> +*     The rest are in place to avoid the struct
> >>> +*     changing size if someone needs them later
> >>> ++ */
> >>> +struct x25_dte_facilities {
> >>> +	unsigned int    calling_len, called_len;
> >>> +	char            calling_ae[20];
> >>> +	char            called_ae[20];
> >>> +	unsigned char   min_throughput;
> >>> +	unsigned short  delay_cumul;
> >>> +	unsigned short  delay_target;
> >>> +	unsigned short  delay_max;
> >>> +	unsigned char   expedited;
> >>> +};
> >>
> >> Why don't you use fixed size members?
> >> And we can eliminate 8bit hole.
> >>
> >> struct x25_dte_facilities {
> >>      u32             calling_len
> >>      u32             called_len;
> >
> > I guess the two above can be 'u8' as they refer to calling_ae and called_ae
> > that at most will be '20'?
> >
> >>      u8              calling_ae[20];
> >>      u8              called_ae[20];
> >
> > - Arnaldo
> 
> At the very least put the 32-bit in the beginning and 8-bit stuff at
> the end so natural alignment occurs where possible.
> 
> Cheers,
> Dick Johnson
> Penguin : Linux version 2.6.13.4 on an i686 machine (5589.46 BogoMips).
> Warning : 98.36% of all statistics are fiction.
> .

Thanks everyone for the feedback, will fix up the alignment.

A quick question on the use of u8 and u32 types in x25.h.

/usr/src/linux-2.6.13.4/include/linux/x25.h and 
/usr/include/linux/x25.h
have been identical between the kernel and glibc-kernheaders type
packages, using u8 and u32 would require extra changes to the userspace
version.

__u32 or unsigned int look to be the norm for other similar headers,
whats the recommended type of types to be used?

Thanks,
Andrew.