Received-SPF: pass (google.com: domain of linux-kernel+bounces-165631-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <26435372-004c-4f8d-b16e-a26a1806147e@nvidia.com>
Date: Wed, 1 May 2024 10:14:11 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] selftests/harness: fix many "format string is empty"
 warnings
To: Nathan Chancellor <nathan@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>, linux-kselftest@vger.kernel.org,
 LKML <linux-kernel@vger.kernel.org>, Valentin Obst <kernel@valentinobst.de>,
 Kees Cook <keescook@chromium.org>, Nick Desaulniers
 <ndesaulniers@google.com>, Justin Stitt <justinstitt@google.com>,
 Bill Wendling <morbo@google.com>
References: <20240501020813.264878-1-jhubbard@nvidia.com>
 <20240501151343.GA1018661@dev-arch.thelio-3990X>
Content-Language: en-US
From: John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20240501151343.GA1018661@dev-arch.thelio-3990X>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?eDRXNHNpNTVzZklqaXZ1aXc5M0x5MkZRaUFJWlRjWWV3YklmWk1mc0orZXND?=
 =?utf-8?B?blJHNVY4OFp1K1JrdVBkTlRKaExzdW1RNmw5UFZQTmczM3IwWkNVY2JUZE5H?=
 =?utf-8?B?UFM4Wm95QWlReStreTlqRWRNcHJyR1JCSVlvQlVGSGRENlBEMjhnUjdQdGdl?=
 =?utf-8?B?SGpwL3hKM1JUTXJMcEdqT0hvYzBINjJTUnpyY0hKdUsvZ0FTdmNHejRUdGdh?=
 =?utf-8?B?aVlEaEJiTmgzcTJBVjRwM1JZTkVkWGFvRkswOUd2WC9pajFHSHRhZUJKUWJZ?=
 =?utf-8?B?RGFHUlkza2c3QTVTWDdITXVEQ1Y0SFNqYXhjVy9FL2VIMFpReTF0ZitZQzE2?=
 =?utf-8?B?MUVzSGQ1U2ZEOFBMbHNNUmZoMmpiUDdGdFhOVmI4Sm5FN2NuUU1XTUZ3UW1W?=
 =?utf-8?B?dFN5bnQwYnM3TjJPSEFzdG02LzhkOXc3U1BJS29rRVY4cldwOUtsN2JWcGNS?=
 =?utf-8?B?bXo2Y2pJL053WTEzRWNpWjJtN2YvNnovYUxsZUtSUzJJZ3ZUcFBhaEJUS2pD?=
 =?utf-8?B?RUJ5OXV3bmNLRlRKUG15aHJZUVJwcG9ycUlNbVNkWG5nclo3OXhtNlMrdFY2?=
 =?utf-8?B?NmVIOStZNGdvOVovVllDS0ZSckxYM0ZDR25yRGxWNUcwb2xHNFpldDJ5bUlL?=
 =?utf-8?B?YUorM3htck9iRjd6M0JYVFhzQytLVVFRdHMveXZSdUVka3RmS0M1dXYveE5k?=
 =?utf-8?B?N3VtckFoY3BPUDR4dDd5UmQzQWY3RWdYdFJYZU1yRExwaUxvSGR3TzRZcGZu?=
 =?utf-8?B?QklrUnp1NFI3eFJxdEFSNS9EVFVzNWVtNFd5dDBsRU12OW5PSFdVbzVMbHpP?=
 =?utf-8?B?Q3l4bWhRaDAxWG84ZjU1SldHNWxEKzNKR0FEL3hmVUN0S3lGRENwV0poRU94?=
 =?utf-8?B?aTlxaVk5R0t4eEFXYWppRmJCbEt3VlovdWx5Y2hyQ25MQ0RjbTNhTVdiRzk1?=
 =?utf-8?B?R2daQTVJYUpWKzhzS3RFSXVkTkhhTml0aVJkRzc2d28yVWR5L3pFRktyd2Rh?=
 =?utf-8?B?UmRFQlRFaVlFRzZ0VjE1MDl0aUxMenk3aGp5NExEcTJjaVZQUXY4cjU2UUpm?=
 =?utf-8?B?VXNmcE1ZbmZ0Z0t4bGhEL0k0R0Q4K2hGb1o0V1c1NytJdnczOUNuZjBjQk5m?=
 =?utf-8?B?MkI4OUxiZjR1MU1JejRkZmFXWTZUUXBnVmJZNUE0dllVbzQzWkJNSzVpenIz?=
 =?utf-8?B?aEZMZnYvcDNRQXhkbTBiMjArK1JWUmJILzJaMWhyKzBwK3ZWOEdPVkdOZ096?=
 =?utf-8?B?ODVNeDEzc1o3Y0wzcThhb0VHdGRKU3BpWi9QQlBsWloxVkowcXoxV0wyR1Vx?=
 =?utf-8?B?SkdkMnlpVmxQNEhJQ0NpaVFwOHJNUjFrbGs1VGo1TlJHZmVySWJpWVYxTjQz?=
 =?utf-8?B?VnJRS01HQ3BCbzdqSkgzNnR2OTRUQy81SGFrYjVHdWpxcCtOb2FDaWNtcHQz?=
 =?utf-8?B?N0MrRWgzYjZoU2FGWEd6MFBtRzE4WXo5bnVrVGdKbkhWSWpGYk9rYTBKQTRz?=
 =?utf-8?B?dlYrV0xKaGlaQlBKd2RzK25mVTZZU29RbFAzNkxvejVFR2p4TDlJdDd3Z0RF?=
 =?utf-8?B?c0VIVTFsTHpqQlY3NjUzbzNtS1pVUG5tc3RoeGZGaWhKZldmelFQZTZlMU5q?=
 =?utf-8?B?aGYyVTJRamwxdjU0ekpsbFBzTE5mS1VUaUltNlRvTmhXS2JRaE5MRkFlQmFw?=
 =?utf-8?B?b0xBY2NzVjVXZUdxWGxoMWEwUjBZUEIrVlZ4YThzM2E4ald1RW5Sd1piSkdp?=
 =?utf-8?B?UmJvMWp0a3YxOHJ0UWxDSW5zdFdEWDJBYnhQNjFYMlh0N3hHSEZFU1RFUlFI?=
 =?utf-8?B?aGxmR1JwQ1BzUlB2N0RPbjg4aUlaNXhCd3hSa0ZBcmhUcFdKNWxvYjF4cjY5?=
 =?utf-8?B?VGJVT1IwY1lIdjFOeTl0VFVzU3NaQm9LZllwRWFwOVdrd2hnV0F3eDZoMkNt?=
 =?utf-8?B?cXhjbU9jVDZFb2VHME9YM2xxTjV2R2ZRNm9oanVXSU4yaGtLL09QQk9kRGgv?=
 =?utf-8?B?NndIS2pQNWVWQ3llVHBDT3pxcXJnYi9BbytUUDNzVDl4MDQwWnVucjZMR0VF?=
 =?utf-8?B?MnpkK0ROUklhdnMxQnhyMDJoekExRUFsa3h4eDcrUEJrUEJBTkdHaVMrc0Nz?=
 =?utf-8?B?S2xHODlpT1NBaVVaNzhTckpmK2FXOXpNVzZRVHN0OFpvYnEwMVZtNmx4RWto?=
 =?utf-8?B?M0E9PQ==?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 517832c5-0fc9-4cd2-6804-08dc6a02239f
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 May 2024 17:14:20.1117
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fbgLK7JSu8481ukBr56SQCeSzROwxbgQBPkHwXkaU0LQQ74Od1rLOTRYRJa8f5GjYRBY3W+48DrZzki4+MJE8A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB5625

On 5/1/24 8:13 AM, Nathan Chancellor wrote:
> Hi John,
> 
> On Tue, Apr 30, 2024 at 07:08:13PM -0700, John Hubbard wrote:
>> In order to build with clang at all, in order to see these symptoms, one
>> must first apply Valentin Obst's build fix for LLVM [1]. Once that is
>> done, then when building with clang, via:
>>
>>      make LLVM=1 -C tools/testing/selftests
>>
>> ...clang emits a "format string is empty" warning. (gcc also emits a
>> similar warning.)
> 
> The warning you are describing here sounds like the same exact one that
> commit caed8eba2215 ("selftests: kselftest_harness: fix Clang warning
> about zero-length format") should have addressed in 6.9-rc5. Is this
> patch actually necessary?

Apparently not!

> 
>> Fix by passing NULL, instead of "", for the msg argument to
> 
> Because this text is describing what was done in caed8eba2215...

Yes. I took a while between discovering the issue, and posting a fix, I
think that's why.

> 
>> ksft_test_result_code(). This removes dozens of warnings and a few
>> errors (some tests have -Werror set).
>>
>> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/
>>
>> Cc: Valentin Obst <kernel@valentinobst.de>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Cc: Nathan Chancellor <nathan@kernel.org>
>> Cc: Shuah Khan <shuah@kernel.org>
>> Cc: Justin Stitt <justinstitt@google.com>
>> Cc: Bill Wendling <morbo@google.com>
>> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>> ---
>>   tools/testing/selftests/kselftest_harness.h | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
>> index d98702b6955d..456b8694e678 100644
>> --- a/tools/testing/selftests/kselftest_harness.h
>> +++ b/tools/testing/selftests/kselftest_harness.h
>> @@ -1207,8 +1207,10 @@ void __run_test(struct __fixture_metadata *f,
>>   	else
>>   		diagnostic = "unknown";
>>   
>> -	ksft_test_result_code(t->exit_code, test_name,
>> -			      diagnostic ? "%s" : NULL, diagnostic);
>> +	if (diagnostic)
>> +		ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic);
>> +	else
>> +		ksft_test_result_code(t->exit_code, test_name, NULL);
> 
> but this diff is not doing that because it is based on a tree that has
> caed8eba2215; instead, it appears to be a completely identical
> transformation?
> 

It's nice to see that someone else had the same fix, anyway. Sorry
about the noise, and yes let's please just drop this one.

thanks,
-- 
John Hubbard
NVIDIA