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] [day] [month] [year] [list]

Date:   Mon, 7 Aug 2023 13:26:25 +0300 (EEST)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     "Wieczor-Retman, Maciej" <maciej.wieczor-retman@...el.com>
cc:     Reinette Chatre <reinette.chatre@...el.com>,
        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()

On Mon, 24 Jul 2023, Wieczor-Retman, Maciej wrote:
> 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?

Yes, it's a great idea. It would be more logical and thus easier to 
comprehend.

Since this already wast applied, it has to be done in another change.


-- 
 i.