Re: sparse context warning problem ...

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 1616 bytes --]

On Sat, 2008-05-10 at 17:24 -0700, David Brownell wrote:
> I just noticed this with some obviously correct code... and verified
> that it's a regression in current GIT version of "sparse" since it
> now rejects code which previously passed just fine.
> 
> The issue is a not-uncommon idiom, where a function must be called
> with a lock held, and briefly drops it.  The way this has previously
> been addressed, originally suggested by Linus and used in various
> places in the kernel (but, I observe, not in the "sparse" internal
> validation test cases) is:
> 
>   static void
>   finish_urb(struct ohci_hcd *ohci, struct urb *urb, int status)
>   __releases(ohci->lock)
>   __acquires(ohci->lock)
>   {
> 	...
>   }
> 
> But current versions of "sparse" complain (wrongly):
> 
>   drivers/usb/host/ohci-q.c:66:2: warning: context imbalance in 'finish_urb': __context__ statement expected different context
>   drivers/usb/host/ohci-q.c:66:2:    context '<noident>': wanted >= 0, got -1
> 
> Presumably this is this a known bug ... is a fix on the way?
> (Meanwhile, I can just ignore this bogus output.)

This is probably my mistake.

However, I took __releases and __acquires to mean that this function
*changed* the context, doing both doesn't really make much sense. I
think the function should actually be declared

static void
finish_urb(...)
__requires(ohci->lock)
{...}

where __requires is (for sparse) defined as

#define __requires(x) __attribute__((context(x,1,1)))

It's probably possible to merge the __acquires and __releases into one
though.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]