bug in checkpath.pl

bug in checkpath.pl

[Holger Schurig]

I have a case where scripts/checkpatch.pl returns a false error. 
First, here is the code:


static int lbs_scan_add_rates_tlv(u8 *tlv)
{
	int i;
	struct mrvlietypes_ratesparamset *rate_tlv =
		(struct mrvlietypes_ratesparamset *) tlv;

	rate_tlv->header.type = cpu_to_le16(TLV_TYPE_RATES);
	tlv += sizeof(rate_tlv->header);
	for (i = 0; i < MAX_RATES; i++) {
		*tlv = lbs_bg_rates[i];
		if (*tlv == 0)
			break;
		if (*tlv == 0x02 || *tlv == 0x04 ||
		    *tlv == 0x0b || *tlv == 0x16)
			*tlv |= 0x80;
		tlv++;
	}
	rate_tlv->header.len = i;
	return sizeof(rate_tlv->header) + i;
}


And here the error from checkpatch.pl:

ERROR: need consistent spacing around '*' (ctx:WxV)
#553: FILE: drivers/net/wireless/libertas/scan.c:438:
+                       *tlv |= 0x80;



This error seems wrong, tlv is a pointer to some u8 value
(a.k.a. unsigned char), and it is very well allowed to
operate on it via *variablename |= 0x80;

Re: bug in checkpath.pl

[Andy Whitcroft]

On Wed, Nov 28, 2007 at 12:07:38PM +0100, Holger Schurig wrote:
> I have a case where scripts/checkpatch.pl returns a false error. 
> First, here is the code:
> 
> 
> static int lbs_scan_add_rates_tlv(u8 *tlv)
> {
> 	int i;
> 	struct mrvlietypes_ratesparamset *rate_tlv =
> 		(struct mrvlietypes_ratesparamset *) tlv;
> 
> 	rate_tlv->header.type = cpu_to_le16(TLV_TYPE_RATES);
> 	tlv += sizeof(rate_tlv->header);
> 	for (i = 0; i < MAX_RATES; i++) {
> 		*tlv = lbs_bg_rates[i];
> 		if (*tlv == 0)
> 			break;
> 		if (*tlv == 0x02 || *tlv == 0x04 ||
> 		    *tlv == 0x0b || *tlv == 0x16)
> 			*tlv |= 0x80;
> 		tlv++;
> 	}
> 	rate_tlv->header.len = i;
> 	return sizeof(rate_tlv->header) + i;
> }
> 
> 
> And here the error from checkpatch.pl:
> 
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #553: FILE: drivers/net/wireless/libertas/scan.c:438:
> +                       *tlv |= 0x80;
> 
> 
> 
> This error seems wrong, tlv is a pointer to some u8 value
> (a.k.a. unsigned char), and it is very well allowed to
> operate on it via *variablename |= 0x80;

Yes that would be wrong.  I believe that this is fixed in the latest
versions.  0.12 should have the fix for this, and it seems to work in
the latest development snapshot.  Can you try the 0.12 and next from the
URL below for me.

    http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/

-apw