On Fri, Feb 24, 2017 at 10:57:07AM -0800, Josh Triplett wrote:
> On Fri, Feb 24, 2017 at 10:07:59AM -0800, Matthew Wilcox wrote:
> > I was recently sent some code that looked like this:
> >
> > int foo()
> > {
> > lock();
> > return bar();
> > unlock();
> > }
> >
> > When you're restructuring code that contains locks, this is a
> > *really* easy mistake to make. I've done it myself. But there's no
> > compiler warning for it! gcc doesn't have it, sparse doesn't have it.
>
> Sparse does have a warning (via -Wcontext) for this, if you annotate
> lock() and unlock() with __acquires(somelock) and __releases(somelock),
> which expand to __attribute__((context(somelock,0,1))) and
> __attribute__((context(somelock,0,1))) respectively. You'll get a
> warning that foo() returns with the lock held.
>
> Not at all perfect, but it does have reasonable handling of
> conditionals, including a way to handle cond_lock().
Ah, yes, thanks. I didn't actually try to compile the patch I was
sent ... I was just bemused that the compiler didn't warn about this
"obvious" wrongness. So I wrote a test-case, which of course didn't have
any lock annotations. rcu_read_lock()/unlock() are correctly annotated
and applying the patch I sent produces a sparse (and not gcc) warning.
So I've asked the submitter to run sparse in future.
(of course, this means they have to ignore all the *other* pre-existing
sparse warnings, but that's not the fault of anyone on this mailing list)
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
