Search Linux Wireless

Re: [PATCH] WAVELAN - compile-time check for struct sizes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Feb 07, 2008 at 10:51:52AM -0500, John W. Linville wrote:
> On Wed, Feb 06, 2008 at 01:59:50PM -0800, Andrew Morton wrote:
> > On Wed, 6 Feb 2008 21:47:47 +0000
> > Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> wrote:
> 
> > > I assume that it's the second BUILD_BUG_ON() which is triggering?
> > 
> > yup.
> > 
> > > Given that:
> > > 
> > > #define MMW_SIZE        37
> > > 
> > > is not a multiple of sizeof(unsigned long) this is hardly surprising.
> > > 
> > > If structures are used to define a layout of something and must not
> > > contain compiler padding, it must be packed.  Given these structures
> > > contain just unsigned char, there's no concerns about >8bit loads
> > > becoming less efficient.
> 
> Does a patch like this suffice?  I haven't checked whether such a
> patch implies that the BUILD_BUG_ON()'s become unnecessary...
> 
> Does anyone actually have this hardware to test?
> 
> ---
> 
> From: John W. Linville <linville@xxxxxxxxxxxxx>
> Subject: [PATCH] wavelan: mark hardware interfacing structures as packed
> 
> Signed-off-by: John W. Linville <linville@xxxxxxxxxxxxx>
> ---
>  drivers/net/wireless/wavelan.h |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/wavelan.h b/drivers/net/wireless/wavelan.h
> index 27172cd..964db3e 100644
> --- a/drivers/net/wireless/wavelan.h
> +++ b/drivers/net/wireless/wavelan.h
> @@ -100,7 +100,7 @@ struct ha_t
>  	unsigned short	ha_piop1;	/* Program I/O Port 1 */
>  	unsigned short	ha_pior2;	/* Program I/O Address Register Port 2 */
>  	unsigned short	ha_piop2;	/* Program I/O Port 2 */
> -};
> +} __attribute__ ((packed));

No need.

>  
>  #define HA_SIZE		16
>  
> @@ -202,7 +202,7 @@ struct psa_t
>    unsigned char	psa_conf_status;	/* [0x3C] Conf Status, bit 0=1:config*/
>    unsigned char	psa_crc[2];		/* [0x3D] CRC-16 over PSA */
>    unsigned char	psa_crc_status;		/* [0x3F] CRC Valid Flag */
> -};
> +} __attribute__ ((packed));
>  
>  #define	PSA_SIZE	64

No need.

>  
> @@ -292,7 +292,7 @@ struct mmw_t
>  #define	MMW_EXT_ANT_INTERNAL	0x00	/* Internal antenna */
>  #define	MMW_EXT_ANT_EXTERNAL	0x03	/* External antenna */
>  #define	MMW_EXT_ANT_IQ_TEST	0x1C	/* IQ test pattern (set to 0) */
> -};
> +} __attribute__ ((packed));;

Needed.

>  
>  #define	MMW_SIZE	37
>  
> @@ -347,7 +347,7 @@ struct mmr_t
>    unsigned char	mmr_unused4[1];		/* unused */
>    unsigned char	mmr_fee_data_l;		/* Read data from EEPROM (low) */
>    unsigned char	mmr_fee_data_h;		/* Read data from EEPROM (high) */
> -};
> +} __attribute__ ((packed));

Needed.
-
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux