Re: [PATCH v4 1/2] platform/x86: Add Uniwill laptop driver

[Nathan Chancellor]

Hi Armin,

On Thu, Oct 02, 2025 at 08:41:19PM +0200, Armin Wolf wrote:
> i think this is a problem inside the clang compiler. I did not encounter this warning when
> build for x86-64 using gcc.

Clang is actually saving you from yourself, it is a bug in GCC that it
does not warn for this:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951

> > vim +1243 drivers/platform/x86/uniwill/uniwill-acpi.c
> > 
> >    1235	
> >    1236	static int uniwill_notifier_call(struct notifier_block *nb, unsigned long action, void *dummy)
> >    1237	{
> >    1238		struct uniwill_data *data = container_of(nb, struct uniwill_data, nb);
> >    1239		struct uniwill_battery_entry *entry;
> >    1240	
> >    1241		switch (action) {
> >    1242		case UNIWILL_OSD_BATTERY_ALERT:
> > > 1243			guard(mutex)(&data->battery_lock);

mutex_unlock() will be called on &data->battery_lock even when the
default case is taken, as demonstrated by the following test case.

> >    1244			list_for_each_entry(entry, &data->batteries, head) {
> >    1245				power_supply_changed(entry->battery);
> >    1246			}
> >    1247	
> >    1248			return NOTIFY_OK;
> >    1249		default:
> >    1250			guard(mutex)(&data->input_lock);
> >    1251			sparse_keymap_report_event(data->input_device, action, 1, true);
> >    1252	
> >    1253			return NOTIFY_OK;
> >    1254		}
> >    1255	}
> >    1256	
> > 
> 

$ cat test.c
#include <stdio.h>

void cleanup_1(int *a) { printf("+ %s(%p)\n", __func__, a); }
void cleanup_2(int *a) { printf("+ %s(%p)\n", __func__, a); }
void cleanup_3(int *a) { printf("+ %s(%p)\n", __func__, a); }

void no_scopes(int a)
{
    printf("%s(%d)\n", __func__, a);
    switch (a) {
    case 1:
        int case_1 __attribute__((cleanup(cleanup_1)));
        return;
    case 2:
        int case_2 __attribute__((cleanup(cleanup_2)));
        return;
    default:
        int case_default __attribute__((cleanup(cleanup_3)));
        return;
    }
}

void with_scopes(int a)
{
    printf("%s(%d)\n", __func__, a);
    switch (a) {
    case 1: {
        int case_1 __attribute__((cleanup(cleanup_1)));
        return;
    }
    case 2: {
        int case_2 __attribute__((cleanup(cleanup_2)));
        return;
    }
    default: {
        int case_default __attribute__((cleanup(cleanup_3)));
        return;
    }
    }
}

int main(void)
{
    no_scopes(1); printf("\n");
    no_scopes(2); printf("\n");
    no_scopes(3); printf("\n");

    with_scopes(1); printf("\n");
    with_scopes(2); printf("\n");
    with_scopes(3);
}

$ gcc -O2 test.c

$ ./a.out
no_scopes(1)
+ cleanup_1(0x7ffea3450c0c)

no_scopes(2)
+ cleanup_2(0x7ffea3450c10)
+ cleanup_1(0x7ffea3450c0c)

no_scopes(3)
+ cleanup_3(0x7ffea3450c14)
+ cleanup_2(0x7ffea3450c10)
+ cleanup_1(0x7ffea3450c0c)

with_scopes(1)
+ cleanup_1(0x7ffea3450c14)

with_scopes(2)
+ cleanup_2(0x7ffea3450c14)

with_scopes(3)
+ cleanup_3(0x7ffea3450c14)

$ clang -O2 test.c
test.c:12:9: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
   12 |         int case_1 __attribute__((cleanup(cleanup_1)));
      |         ^
test.c:15:9: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
   15 |         int case_2 __attribute__((cleanup(cleanup_2)));
      |         ^
test.c:18:9: warning: label followed by a declaration is a C23 extension [-Wc23-extensions]
   18 |         int case_default __attribute__((cleanup(cleanup_3)));
      |         ^
test.c:17:5: error: cannot jump from switch statement to this case label
   17 |     default:
      |     ^
test.c:15:13: note: jump bypasses initialization of variable with __attribute__((cleanup))
   15 |         int case_2 __attribute__((cleanup(cleanup_2)));
      |             ^
test.c:12:13: note: jump bypasses initialization of variable with __attribute__((cleanup))
   12 |         int case_1 __attribute__((cleanup(cleanup_1)));
      |             ^
test.c:14:5: error: cannot jump from switch statement to this case label
   14 |     case 2:
      |     ^
test.c:12:13: note: jump bypasses initialization of variable with __attribute__((cleanup))
   12 |         int case_1 __attribute__((cleanup(cleanup_1)));
      |             ^
3 warnings and 2 errors generated.

https://godbolt.org/z/1Tx7Gj1xf

I would add the scoping to the case labels or use scoped_guard() to
avoid this, which would also avoid the instances of -Wc23-extensions.

Cheers,
Nathan