linux-kernel - Re: [PATCH v4 07/19] selftests/resctrl: Refactor remount_resctrl(bool mum_resctrlfs) to mount

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <903b5c02-7d7b-11c7-167b-c4f0f3862ae0@intel.com>
Date:   Mon, 24 Jul 2023 09:12:27 +0200
From:   "Wieczor-Retman, Maciej" <maciej.wieczor-retman@...el.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        "Reinette Chatre" <reinette.chatre@...el.com>
CC:     <linux-kselftest@...r.kernel.org>, Shuah Khan <shuah@...nel.org>,
        "Shaopeng Tan" <tan.shaopeng@...fujitsu.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 07/19] selftests/resctrl: Refactor remount_resctrl(bool
 mum_resctrlfs) to mount_resctrl()

Hi!

On 14.07.2023 13:03, Ilpo Järvinen wrote:
> On Thu, 13 Jul 2023, Reinette Chatre wrote:
>> On 7/13/2023 6:19 AM, Ilpo Järvinen wrote:
>>> -int remount_resctrlfs(bool mum_resctrlfs)
>>> +int mount_resctrlfs(void)
>>>  {
>>> -	char mountpoint[256];
>>>  	int ret;
>>>  
>>> -	ret = find_resctrl_mount(mountpoint);
>>> -	if (ret)
>>> -		strcpy(mountpoint, RESCTRL_PATH);
>>> -
>>> -	if (!ret && mum_resctrlfs && umount(mountpoint))
>>> -		ksft_print_msg("Fail: unmounting \"%s\"\n", mountpoint);
>>> -
>>> -	if (!ret && !mum_resctrlfs)
>>> -		return 0;
>>> +	ret = find_resctrl_mount(NULL);
>>> +	if (!ret)
>>> +		return -1;
>>
>> This treats "ret == 0" as a failure. What about -ENXIO? It seems to
>> me that only "ret == -ENOENT" is "success".
> 
> Yes, it's a good catch.
> 

I had an idea about a small redesign of find_resctrl_mount
return values so it is easier to see what the function tries
to accomplish.

When there is an error (-ENXIO for example) it could 
return the negative error value. When no mount is found
it could return a zero (instead of the -ENOENT error code).
Finally when a mount point was found it could return a positive
value (for example return 1). This way errors could be 
separate from regular return values and in my opinion the
function logic would be more transparent.

What do you think about it?

Kind regards
Maciej Wieczór-Retman