Hi Andy, On 5/19/2020 9:07 AM, Andy Shevchenko wrote: > On Tue, May 19, 2020 at 08:50:22AM -0700, Reinette Chatre wrote: >> On 5/19/2020 1:28 AM, Andy Shevchenko wrote: >>> On Tue, May 19, 2020 at 2:50 AM Reinette Chatre >>> <reinette.cha...@intel.com> wrote: > > ... > >>>> + ret = sysfs_match_string(rdt_mode_str, buf); >>>> + if (ret < 0) { >>>> + rdt_last_cmd_puts("Unknown or unsupported mode\n"); >>>> + ret = -EINVAL; >>>> + goto out; >>>> + } >> >> From your previous email ... >> >>>> + ret = sysfs_match_string(rdt_mode_str, buf); >>>> + if (ret < 0) { >>>> + rdt_last_cmd_puts("Unknown or unsupported mode\n"); >>> >>>> + ret = -EINVAL; >>> >>> This is redundant. >> >> I understand that shadowing an error code is generally of concern. In >> this case the error code is set to -EINVAL to ensure that it is the same >> error code that was returned to user space originally and will continue >> to be so no matter what changes may come to sysfs_match_string(). > > It returns -EINVAL and if that will be ever changed this driver would be one > of > hundreds who suffers.
Not if we keep this change ... but that is no longer of concern with the removal of the check as you propose later. > > ... > >>> Can't we unify latter with a former like ... > >> This would have been ideal if done from the start but currently "0" is >> returned if the current mode is pseudo-locked and user attempts to >> change the mode to pseudo-locked. Thus, to maintain the current user >> interface the check if user wants to set pseudo-locked mode is moved >> after the check if new mode is same as existing mode and thus not >> unified because that will result in an error returned always when user >> requests pseudo-locked mode. > > Ah, I see now. > > But we can then drop the check from sysfs_match_string() returned value, like > > user_m = sysfs_match_string(); > if (...) { > ... > } else { // w/o even checking for the PSEUDO_LOCKED > ... > goto out; > } > > Can we? > Yes, we can. Will do. Reinette