Re: [PATCH v4 07/19] selftests/resctrl: Refactor remount_resctrl(bool mum_resctrlfs) to mount_resctrl()

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1629 bytes --]

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.