Re: [RFC PATCH] cleanup: make scoped_guard() to be return-friendly

[Dan Carpenter <dan.carpenter@linaro.org>]

On Mon, Sep 30, 2024 at 01:30:58PM +0200, Przemek Kitszel wrote:
> > but your macro is wrong and will need to be re-written
> 
> could you please elaborate here?

- 		__guard_ptr(_name)(&scope) && !done;
+		__guard_ptr(_name)(&scope), 1;     \

The __guard_ptr(_name)(&scope) check is checking whether lock function
succeeded.  With the new macro we only use scoped_guard() for locks which can't
fail.  We can (basically must) remove the __guard_ptr(_name)(&scope) check since
we're ignoring the result.

There are only four drivers which rely on conditional scoped_guard() locks.

$ git grep scoped_guard | egrep '(try|_intr)'
drivers/input/keyboard/atkbd.c: scoped_guard(mutex_intr, &atkbd->mutex) {
drivers/input/touchscreen/tsc200x-core.c:       scoped_guard(mutex_try, &ts->mutex) {
drivers/input/touchscreen/wacom_w8001.c:        scoped_guard(mutex_intr, &w8001->mutex) {
drivers/platform/x86/ideapad-laptop.c:  scoped_guard(mutex_intr, &dytc->mutex) {

This change breaks the drivers at runtime, but you need to ensure that drivers
using the old API will break at compile time so that people don't introduce new
bugs during the transition.  In other words, you will need to update the
DEFINE_GUARD_COND() stuff as well.

$ git grep DEFINE_GUARD_COND
include/linux/cleanup.h: * DEFINE_GUARD_COND(name, ext, condlock)
include/linux/cleanup.h:#define DEFINE_GUARD_COND(_name, _ext, _condlock) \
include/linux/iio/iio.h:DEFINE_GUARD_COND(iio_claim_direct, _try, ({
include/linux/mutex.h:DEFINE_GUARD_COND(mutex, _try, mutex_trylock(_T))
include/linux/mutex.h:DEFINE_GUARD_COND(mutex, _intr, mutex_lock_interruptible(_T) == 0)
include/linux/rwsem.h:DEFINE_GUARD_COND(rwsem_read, _try, down_read_trylock(_T))
include/linux/rwsem.h:DEFINE_GUARD_COND(rwsem_read, _intr, down_read_interruptible(_T) == 0)
include/linux/rwsem.h:DEFINE_GUARD_COND(rwsem_write, _try, down_write_trylock(_T))

I propose that you use scoped_try_guard() and scoped_guard_interruptible() to
support conditional locking.  Creating different macros for conditional locks is
the only way you can silence your GCC warnings and it makes life easier for me
as a static checker developer as well.  It's probably more complicated than I
have described so I'll leave that up to you, but this first draft doesn't work.

regards,
dan carpenter