linux-kernel - Re: [PATCH v3 01/29] selftests/resctrl: Convert perror() to ksft_perror() or ksft_print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a8fa9a13-d49f-4f16-b99e-a128a5feedb2@intel.com>
Date:   Thu, 14 Dec 2023 10:10:29 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC:     <linux-kselftest@...r.kernel.org>, Shuah Khan <shuah@...nel.org>,
        Shaopeng Tan <tan.shaopeng@...fujitsu.com>,
        Maciej Wieczór-Retman 
        <maciej.wieczor-retman@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 01/29] selftests/resctrl: Convert perror() to
 ksft_perror() or ksft_print_msg()

Hi Ilpo,

On 12/14/2023 2:12 AM, Ilpo Järvinen wrote:
> On Wed, 13 Dec 2023, Reinette Chatre wrote:
> 
>> Hi Ilpo,
>>
>> On 12/11/2023 4:17 AM, Ilpo Järvinen wrote:
>>> The resctrl selftest code contains a number of perror() calls. Some of
>>> them come with hash character and some don't. The kselftest framework
>>> provides ksft_perror() that is compatible with test output formatting
>>> so it should be used instead of adding custom hash signs.
>>>
>>> Some perror() calls are too far away from anything that sets error.
>>> For those call sites, ksft_print_msg() must be used instead.
>>>
>>> Convert perror() to ksft_perror() or ksft_print_msg().
>>>
>>> Other related changes:
>>> - Remove hash signs
>>> - Remove trailing stops & newlines from ksft_perror()
>>> - Add terminating newlines for converted ksft_print_msg()
>>> - Use consistent capitalization
>>>
>>
>> Another great cleanup. Also thanks for fixing some non-sensical messages.
>>
>> ...
>>
>>> @@ -149,7 +149,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>>>  	param.num_of_runs = 0;
>>>  
>>>  	if (pipe(pipefd)) {
>>> -		perror("# Unable to create pipe");
>>> +		ksft_perror("Unable to create pipe");
>>>  		return errno;
>>>  	}
>>>  
>>> @@ -185,7 +185,7 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type)
>>>  			 * Just print the error message.
>>>  			 * Let while(1) run and wait for itself to be killed.
>>>  			 */
>>> -			perror("# failed signaling parent process");
>>> +			ksft_perror("Failed signaling parent process");
>>>  
>>
>> Partial writes are not actually errors and it cannot be expected that errno be set
>> in these cases. In these cases I think ksft_print_msg() would be more appropriate.
> 
> I can change those to use print instead although I don't think these will
> fail for other reasons than a real error as the pipe should be empty and 
> only single byte is written to it.
> 

Apologies, I did not pay attention to the actual size of the message. Yes,
leaving it as ksft_perror() is reasonable.

Reinette