Re: [PATCH 2/2] x86/resctrl: Use appropriate API for strings terminated by newline

Andy Shevchenko
2020-04-03T07:28:13Z

On Fri, Apr 3, 2020 at 12:54 AM Reinette Chatre
<reinette.chatre@intel.com> wrote:
> On 4/2/2020 6:06 AM, Andy Shevchenko wrote:
> > On Wed, Apr 01, 2020 at 11:30:48AM -0700, Reinette Chatre wrote:

...

> >> @@ -1412,11 +1412,11 @@ static ssize_t rdtgroup_mode_write(struct kernfs_open_file *of,
> >> struct rdtgroup *rdtgrp;
> >> enum rdtgrp_mode mode;
> >> int ret = 0;
> >> + int user_m;
> >>
>
> >
> > ...and forgot to mention this...
> >
> > int user_m;
> > int ret;
> >
>
> >
> >> /* Valid input requires a trailing newline */
> >> if (nbytes == 0 || buf[nbytes - 1] != '\n')
> >> return -EINVAL;
> >> - buf[nbytes - 1] = '\0';
> >
> > The above test is not needed and comment now is misleading.
> > WRT nbytes I believe that kernel fs code checks for that.

This module provides it's own kernfs_ops...

> If nbytes is 0 it is still passed to this function. You are correct that
> those tests are not needed though (if nbytes is 0 then
> sysfs_match_string() will not find a match and return EINVAL via that path).
>
> Thank you for catching this. I'll remove those unnecessary checks.

...which means that nbytes == 0 is a valid check. Please keep it
there. It will protect from unnecessary locking and loading CPU for
nothing.

--
With Best Regards,
Andy Shevchenko