lkml.org 
[lkml]   [2020]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRE: [PATCH V3 4/4] x86/resctrl: Use appropriate API for strings terminated by newline
    From
    Date


    > -----Original Message-----
    > From: Reinette Chatre <reinette.chatre@intel.com>
    > Sent: Wednesday, May 6, 2020 6:50 PM
    > To: tglx@linutronix.de; fenghua.yu@intel.com; bp@alien8.de;
    > tony.luck@intel.com
    > Cc: kuo-lang.tseng@intel.com; ravi.v.shankar@intel.com; mingo@redhat.com;
    > Moger, Babu <Babu.Moger@amd.com>; hpa@zytor.com; x86@kernel.org;
    > linux-kernel@vger.kernel.org; Reinette Chatre <reinette.chatre@intel.com>;
    > Andy Shevchenko <andriy.shevchenko@linux.intel.com>
    > Subject: [PATCH V3 4/4] x86/resctrl: Use appropriate API for strings terminated
    > by newline
    >
    > The user input to files in the resctrl filesystem are expected to be
    > terminated with a newline. Testing the user input includes a test for
    > the presence of a newline and then replacing the newline with NUL
    > byte followed by comparison using strcmp().
    >
    > sysfs_streq() exists to test if strings are equal, treating both NUL and
    > newline-then-NUL as equivalent string terminations. Even more,
    > sysfs_match_string() exists to match a given string in an array using
    > sysfs_streq().
    >
    > Replace existing strcmp() comparisons of strings that are terminated
    > with a newline with more appropriate sysfs_streq() via the
    > sysfs_match_string() API that can perform the match across the different
    > mode strings that are already maintained in an array.
    >
    > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
    > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
    > ---
    > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 32 ++++++++++++++------------
    > 1 file changed, 17 insertions(+), 15 deletions(-)
    >
    > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
    > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
    > index c60a3b307f7d..e70694892c02 100644
    > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
    > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
    > @@ -1408,12 +1408,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;
    > + int ret;

    This is bit confusing here. Mode and user_m should have been of same type.
    You can define user_m to user_mode to be very clear.

    You can completely remove "mode" and directly use rdtgrp->mode instead. It
    is left to you.

    > > - /* Valid input requires a trailing newline */
    > - if (nbytes == 0 || buf[nbytes - 1] != '\n')
    > + if (nbytes == 0)
    > return -EINVAL;
    > - buf[nbytes - 1] = '\0';
    >
    > rdtgrp = rdtgroup_kn_lock_live(of->kn);
    > if (!rdtgrp) {
    > @@ -1425,11 +1424,17 @@ static ssize_t rdtgroup_mode_write(struct
    > kernfs_open_file *of,
    >
    > mode = rdtgrp->mode;
    >
    > - if ((!strcmp(buf, "shareable") && mode == RDT_MODE_SHAREABLE) ||
    > - (!strcmp(buf, "exclusive") && mode == RDT_MODE_EXCLUSIVE) ||
    > - (!strcmp(buf, "pseudo-locksetup") &&
    > - mode == RDT_MODE_PSEUDO_LOCKSETUP) ||
    > - (!strcmp(buf, "pseudo-locked") && mode ==
    > RDT_MODE_PSEUDO_LOCKED))
    > + ret = sysfs_match_string(rdt_mode_str, buf);
    > + if (ret < 0) {
    > + rdt_last_cmd_puts("Unknown or unsupported mode\n");
    > + goto out;
    > + }
    > +
    > + user_m = ret;
    > + ret = 0;
    > +
    > + /* Do nothing and return success if user asks for current mode */
    > + if (user_m == mode)
    > goto out;
    >
    > if (mode == RDT_MODE_PSEUDO_LOCKED) {
    > @@ -1438,14 +1443,14 @@ static ssize_t rdtgroup_mode_write(struct
    > kernfs_open_file *of,
    > goto out;
    > }
    >
    > - if (!strcmp(buf, "shareable")) {
    > + if (user_m == RDT_MODE_SHAREABLE) {
    > if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
    > ret = rdtgroup_locksetup_exit(rdtgrp);
    > if (ret)
    > goto out;
    > }
    > rdtgrp->mode = RDT_MODE_SHAREABLE;
    > - } else if (!strcmp(buf, "exclusive")) {
    > + } else if (user_m == RDT_MODE_EXCLUSIVE) {
    > if (!rdtgroup_mode_test_exclusive(rdtgrp)) {
    > ret = -EINVAL;
    > goto out;
    > @@ -1456,14 +1461,11 @@ static ssize_t rdtgroup_mode_write(struct
    > kernfs_open_file *of,
    > goto out;
    > }
    > rdtgrp->mode = RDT_MODE_EXCLUSIVE;
    > - } else if (!strcmp(buf, "pseudo-locksetup")) {
    > + } else if (user_m == RDT_MODE_PSEUDO_LOCKSETUP) {
    > ret = rdtgroup_locksetup_enter(rdtgrp);
    > if (ret)
    > goto out;
    > rdtgrp->mode = RDT_MODE_PSEUDO_LOCKSETUP;
    > - } else {
    > - rdt_last_cmd_puts("Unknown or unsupported mode\n");
    > - ret = -EINVAL;
    > }
    >
    > out:
    > --
    > 2.21.0

    \
     
     \ /
      Last update: 2020-05-14 21:06    [W:3.400 / U:0.992 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site