public inbox for llvm@lists.linux.dev
 help / color / mirror / Atom feed
* Re: [PATCH v3 4/9] platform/wmi: Add kunit test for the string conversion code
       [not found] ` <20260109214619.7289-5-W_Armin@gmx.de>
@ 2026-01-22 23:45   ` Nathan Chancellor
  2026-01-23 19:17     ` Armin Wolf
  0 siblings, 1 reply; 2+ messages in thread
From: Nathan Chancellor @ 2026-01-22 23:45 UTC (permalink / raw)
  To: Armin Wolf
  Cc: hansg, ilpo.jarvinen, platform-driver-x86, linux-kernel, linux,
	Dell.Client.Kernel, corbet, linux-doc, llvm

Hi Armin,

On Fri, Jan 09, 2026 at 10:46:14PM +0100, Armin Wolf wrote:
> The string conversion frunctions provided by the WMI driver core
> have no dependencies on the remaining WMI API, making them suitable
> for unit tests.
> 
> Implement such a unit test using kunit. Those unit tests verify that
> converting between WMI strings and UTF8 strings works as expected.
> They also verify that edge cases are handled correctly.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
...
> +++ b/drivers/platform/wmi/tests/string_kunit.c
...
> +static const u8 oversized_test_utf8_string[] = "TEST!";
...
> +static void wmi_string_to_utf8s_oversized_test(struct kunit *test)
> +{
> +	u8 result[sizeof(oversized_test_utf8_string)];
> +	ssize_t ret;
> +
> +	ret = wmi_string_to_utf8s(&oversized_test_wmi_string, result, sizeof(result));
> +
> +	KUNIT_EXPECT_EQ(test, ret, sizeof(test_utf8_string) - 1);
> +	KUNIT_EXPECT_MEMEQ(test, result, test_utf8_string, sizeof(test_utf8_string));
> +}

After this change in -next as commit 0e1a8143e797 ("platform/wmi: Add
kunit test for the string conversion code"), I am seeing a warning from
clang around oversized_test_utf8_string:

  drivers/platform/wmi/tests/string_kunit.c:108:17: error: variable 'oversized_test_utf8_string' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]
    108 | static const u8 oversized_test_utf8_string[] = "TEST!";
        |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~

oversized_test_utf8_string is only used in sizeof() in
wmi_string_to_utf8s_oversized_test(). clang warns because sizeof() is
evaluated at compile time, so oversized_test_utf8_string won't end up in
the final binary. This is typically a bug since the developer may have
intended to use the variable elsewhere but I was not able to easily
determine that in this case.

If it is intentional that this variable is only needed in sizeof(), it
could either be marked with __maybe_unused or eliminated in favor of a
direct 'sizeof("TEST!")', depending on maintainer/developer preference.
I am happy to send a patch.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v3 4/9] platform/wmi: Add kunit test for the string conversion code
  2026-01-22 23:45   ` [PATCH v3 4/9] platform/wmi: Add kunit test for the string conversion code Nathan Chancellor
@ 2026-01-23 19:17     ` Armin Wolf
  0 siblings, 0 replies; 2+ messages in thread
From: Armin Wolf @ 2026-01-23 19:17 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: hansg, ilpo.jarvinen, platform-driver-x86, linux-kernel, linux,
	Dell.Client.Kernel, corbet, linux-doc, llvm

Am 23.01.26 um 00:45 schrieb Nathan Chancellor:

> Hi Armin,
>
> On Fri, Jan 09, 2026 at 10:46:14PM +0100, Armin Wolf wrote:
>> The string conversion frunctions provided by the WMI driver core
>> have no dependencies on the remaining WMI API, making them suitable
>> for unit tests.
>>
>> Implement such a unit test using kunit. Those unit tests verify that
>> converting between WMI strings and UTF8 strings works as expected.
>> They also verify that edge cases are handled correctly.
>>
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ...
>> +++ b/drivers/platform/wmi/tests/string_kunit.c
> ...
>> +static const u8 oversized_test_utf8_string[] = "TEST!";
> ...
>> +static void wmi_string_to_utf8s_oversized_test(struct kunit *test)
>> +{
>> +	u8 result[sizeof(oversized_test_utf8_string)];
>> +	ssize_t ret;
>> +
>> +	ret = wmi_string_to_utf8s(&oversized_test_wmi_string, result, sizeof(result));
>> +
>> +	KUNIT_EXPECT_EQ(test, ret, sizeof(test_utf8_string) - 1);
>> +	KUNIT_EXPECT_MEMEQ(test, result, test_utf8_string, sizeof(test_utf8_string));
>> +}
> After this change in -next as commit 0e1a8143e797 ("platform/wmi: Add
> kunit test for the string conversion code"), I am seeing a warning from
> clang around oversized_test_utf8_string:
>
>    drivers/platform/wmi/tests/string_kunit.c:108:17: error: variable 'oversized_test_utf8_string' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]
>      108 | static const u8 oversized_test_utf8_string[] = "TEST!";
>          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
> oversized_test_utf8_string is only used in sizeof() in
> wmi_string_to_utf8s_oversized_test(). clang warns because sizeof() is
> evaluated at compile time, so oversized_test_utf8_string won't end up in
> the final binary. This is typically a bug since the developer may have
> intended to use the variable elsewhere but I was not able to easily
> determine that in this case.
>
> If it is intentional that this variable is only needed in sizeof(), it
> could either be marked with __maybe_unused or eliminated in favor of a
> direct 'sizeof("TEST!")', depending on maintainer/developer preference.
> I am happy to send a patch.
>
> Cheers,
> Nathan

Good catch, it seems that i forgot to add the test case that was supposed to use oversized_test_utf8_string.
I will send a patch that adds this missing test case, fixing this warning in the process.

Thanks,
Armin Wolf


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-01-23 19:17 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260109214619.7289-1-W_Armin@gmx.de>
     [not found] ` <20260109214619.7289-5-W_Armin@gmx.de>
2026-01-22 23:45   ` [PATCH v3 4/9] platform/wmi: Add kunit test for the string conversion code Nathan Chancellor
2026-01-23 19:17     ` Armin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox