Re: [PATCH] d80211: fix sparse warnings

[Pavel Roskin <proski@gnu.org>]

On Mon, 2007-02-26 at 23:59 +0100, Johannes Berg wrote:
> This fixes some sparse warnings in d80211.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> 
> ---
> A few warnings remain:
> 
> net/d80211/ieee80211.c:820:37: warning: potentially expensive pointer subtraction

That's subtraction of pointers to types with sizes that are not power of
two.  This could lead to expensive division operations.  In may cases,
this warning indicates that using array indices instead of pointers
would be more effective (and probably safer).

> net/d80211/ieee80211_ioctl.c:779:4: warning: incorrect type in argument 6 (different signedness)
> net/d80211/ieee80211_ioctl.c:779:4:    expected int *err
> net/d80211/ieee80211_ioctl.c:779:4:    got unsigned int *<noident>

ieee80211_set_encryption() expects a pointer to (signed) integer, be the
code is giving it a pointer to u32.  Please decide what type "err"
should have, and change the function accordingly.

I think "u32" would be better, since it's a part of the hostapd
compatibility code and we don't want to touch the userspace now.  This
code is going to be obsoleted with the rest of hostapd interface.  For
now, it's better to do exactly what the userspace expects.

> net/d80211/sta_info.c:232:3: warning: context imbalance in 'sta_info_free' - unexpected unlock
> 
> The last of these is bogus, the code is perfectly fine. The other two I
> don't understand.

Great catch!  I have reduced this to a simple test case where sparse
behaves unreasonably, and I'm going to send it to sparse developers as a
bug report.

In case you are wondering, the test case is:

void my_lock(void) __attribute__ ((context(lock, 0, 1)));
void my_unlock(void) __attribute__ ((context(lock, 1, 0)));
void foo(void);
static void sta_info_free(int locked)
{
	if (locked)
		my_lock();
	foo();
	if (locked)
		my_unlock();
}

-- 
Regards,
Pavel Roskin