[PATCH] bcm43xx: iw_priv_args names should be <16 characters

[PATCH] bcm43xx: iw_priv_args names should be <16 characters

[Erik Mouw]

The room for the names in bcm43xx_priv_wx_args[] are IFNAMSIZ long and
IFNAMSIZ is defined as 16, so the names in bcm43xx_priv_wx_args should
be 15 characters (16 including the trailing \0). This patch fixes that
for the "set_shortpreambl", "get_shortpreambl", "set_swencryption", and
"get_swencryption" private calls. Patch is against 2.6.17-rc1.

Signed-off-by: Erik Mouw <erik@harddisk-recovery.com>

diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
index 3daee82..3edbb48 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
@@ -962,22 +962,22 @@ static const struct iw_priv_args bcm43xx
 	{
 		.cmd		= PRIV_WX_SET_SHORTPREAMBLE,
 		.set_args	= IW_PRIV_TYPE_INT | IW_PRIV_SIZE_FIXED | 1,
-		.name		= "set_shortpreambl",
+		.name		= "set_shortpreamb",
 	},
 	{
 		.cmd		= PRIV_WX_GET_SHORTPREAMBLE,
 		.get_args	= IW_PRIV_TYPE_CHAR | IW_PRIV_SIZE_FIXED | MAX_WX_STRING,
-		.name		= "get_shortpreambl",
+		.name		= "get_shortpreamb",
 	},
 	{
 		.cmd		= PRIV_WX_SET_SWENCRYPTION,
 		.set_args	= IW_PRIV_TYPE_INT | IW_PRIV_SIZE_FIXED | 1,
-		.name		= "set_swencryption",
+		.name		= "set_swencrypt",
 	},
 	{
 		.cmd		= PRIV_WX_GET_SWENCRYPTION,
 		.get_args	= IW_PRIV_TYPE_CHAR | IW_PRIV_SIZE_FIXED | MAX_WX_STRING,
-		.name		= "get_swencryption",
+		.name		= "get_swencrypt",
 	},
 	{
 		.cmd		= PRIV_WX_SPROM_WRITE,

Re: [PATCH] bcm43xx: iw_priv_args names should be <16 characters

[Stephen Hemminger]

On Thu, 13 Apr 2006 15:02:27 +0200
Erik Mouw <erik@harddisk-recovery.com> wrote:

> The room for the names in bcm43xx_priv_wx_args[] are IFNAMSIZ long and
> IFNAMSIZ is defined as 16, so the names in bcm43xx_priv_wx_args should
> be 15 characters (16 including the trailing \0). This patch fixes that
> for the "set_shortpreambl", "get_shortpreambl", "set_swencryption", and
> "get_swencryption" private calls. Patch is against 2.6.17-rc1.
> 
> Signed-off-by: Erik Mouw <erik@harddisk-recovery.com>

Shouldn't compiler have gagged on this?

Re: [PATCH] bcm43xx: iw_priv_args names should be <16 characters

[Erik Mouw]

On Thu, Apr 13, 2006 at 10:29:19AM -0700, Stephen Hemminger wrote:
> On Thu, 13 Apr 2006 15:02:27 +0200
> Erik Mouw <erik@harddisk-recovery.com> wrote:
> 
> > The room for the names in bcm43xx_priv_wx_args[] are IFNAMSIZ long and
> > IFNAMSIZ is defined as 16, so the names in bcm43xx_priv_wx_args should
> > be 15 characters (16 including the trailing \0). This patch fixes that
> > for the "set_shortpreambl", "get_shortpreambl", "set_swencryption", and
> > "get_swencryption" private calls. Patch is against 2.6.17-rc1.
> > 
> > Signed-off-by: Erik Mouw <erik@harddisk-recovery.com>
> 
> Shouldn't compiler have gagged on this?

Apparently not. At least the compiler I use doesn't warn about it (gcc
version 3.3.5 (Debian 1:3.3.5-13)).

Linus, this might be be something for sparse to check:

  struct mystruct {
	char name[16];
  };

  mystruct ms = { .name = "muchlongerthan16characters" };


Erik

-- 
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands

Re: [PATCH] bcm43xx: iw_priv_args names should be <16 characters

[Mitchell Blank Jr]

Erik Mouw wrote:
> On Thu, Apr 13, 2006 at 10:29:19AM -0700, Stephen Hemminger wrote:
> > Shouldn't compiler have gagged on this?
> 
> Apparently not. At least the compiler I use doesn't warn about it (gcc
> version 3.3.5 (Debian 1:3.3.5-13)).
> 
> Linus, this might be be something for sparse to check:
> 
>   struct mystruct {
> 	char name[16];
>   };
> 
>   mystruct ms = { .name = "muchlongerthan16characters" };

If you actually try that example you'd see that gcc (even really old
versions) will warn you about that code.

What gcc WON'T warn about is if everything but the final '\0' fits in
the array.

So the code:
	struct mystruct { char name[16]; };
	struct mystruct exact_length = { .name = "0123456789abcdef" };
	struct mystruct one_too_long = { .name = "0123456789abcdefg" };

Produces:
	a.c:3: warning: initializer-string for array of chars is too long
	a.c:3: warning: (near initialization for 'one_too_long.name')

I believe this is intentional and in line with the C specs.  It's also
useful since it allows you to define macros like:

	#define DECLARE_NON_TERMINATED_STRING(name, val) \
			const name[sizeof(val "") - 1] = val;

Making sparse a bit stricter than gcc in this case might not be a bad idea,
though.

-Mitch