[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <402f254d-7217-43e8-867f-66daab3ead86@gmx.de>
Date: Sun, 5 Oct 2025 20:06:57 +0200
From: Armin Wolf <W_Armin@....de>
To: Nathan Chancellor <nathan@...nel.org>
Cc: kernel test robot <lkp@...el.com>, ilpo.jarvinen@...ux.intel.com,
hdegoede@...hat.com, chumuzero@...il.com, corbet@....net, cs@...edo.de,
wse@...edocomputers.com, ggo@...edocomputers.com, llvm@...ts.linux.dev,
oe-kbuild-all@...ts.linux.dev, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
rdunlap@...radead.org, alok.a.tiwari@...cle.com, linux-leds@...r.kernel.org,
lee@...nel.org, pobrn@...tonmail.com
Subject: Re: [PATCH v4 1/2] platform/x86: Add Uniwill laptop driver
Am 03.10.25 um 01:36 schrieb 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
Oh my, i didn't expect that. Thank you for explaining this issue to to me,
it seems that i still have much to learn.
Thanks,
Armin Wolf
>>> 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
>
Powered by blists - more mailing lists