Re: [PATCH v5 1/5] selftests/mm: make file helpers return errors

[John Hubbard <jhubbard@nvidia.com>]

On 7/21/26 10:24 PM, Sarthak Sharma wrote:
> Hi David and Mark!
> 
> On 7/21/26 8:24 PM, David Hildenbrand (Arm) wrote:
>> On 7/21/26 15:48, Mark Brown wrote:
>>> On Tue, Jul 21, 2026 at 03:39:09PM +0200, David Hildenbrand (Arm) wrote:
>>>> On 7/16/26 14:32, Sarthak Sharma wrote:
>>>
>>>>>  	fd = open(path, O_RDONLY);
>>>>> -	if (fd == -1)
>>>>> -		return 0;
>>>>> +	if (fd == -1) {
>>>>> +		int err = errno;
>>>>> +
>>>>> +		printf("# %s: %s (%d)\n", path, strerror(err), err);
>>>
>>>> Shouldn't we be using
>>>
>>>> ksft_print_msg()
>>>
>>>> That does the magic "# " for us.
>>>
>>> Or ksft_perror().
>>
>>
>> Right, and if we want to move this to tools/lib/mm later, maybe we can just not
>> print anything and instead only return expressive errnos.
>>
> 
> Yes, we are not using ksft_* prints here because these functions move to
> tools/lib/mm later in the series.
> 
> Printing errors from the helpers was suggested by Mike [1]. I also think
> this is useful since the diagnostic handling stays at one place, instead
> of duplicating it in every caller.

This is a common pitfall. In fact, the lower ("leaf") routines, *especially*
library-like functions that are called by disparate callers, should be
usually be silent. 

That's because printing is a policy choice, and libraries must refrain
from imposing that policy onto their callers.

Higher up the call stack, the code has more context, and is able to
make more informed choices about whether or not to print something.
And when or if they do print, they may include some of that extra
context, and so you can now see that the printing code is potentially
different at each call site, and now you haven't actually saved any
duplication after all by trying to centralize it.> 
> If we want helpers to be silent, I can change them to return errors and
> let the callers handle the diagnostics. Please let me know how I should
> proceed here.
> 
> [1]
> https://lore.kernel.org/all/178048086818.472368.16811711474077698399.b4-review@b4/

thanks,
-- 
John Hubbard