From: Reinette Chatre <reinette.chatre@intel.com>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: tglx@linutronix.de, fenghua.yu@intel.com, bp@alien8.de,
tony.luck@intel.com, kuo-lang.tseng@intel.com,
ravi.v.shankar@intel.com, mingo@redhat.com, babu.moger@amd.com,
hpa@zytor.com, x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH V4 4/4] x86/resctrl: Use appropriate API for strings terminated by newline
Date: Mon, 18 May 2020 09:06:00 -0700 [thread overview]
Message-ID: <060e3cf6-8fcb-6b43-01ef-63ef0a5ff64a@intel.com> (raw)
In-Reply-To: <20200518115026.GA1634618@smile.fi.intel.com>
Hi Andy,
On 5/18/2020 4:50 AM, Andy Shevchenko wrote:
> On Sat, May 16, 2020 at 11:28:41AM -0700, Reinette Chatre wrote:
>> 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.
>
> Sorry for late comment, but just have noticed...
No problem. I do appreciate your feedback because it helps me to improve
the code.
>
>> if (mode == RDT_MODE_PSEUDO_LOCKED) {
>> @@ -1445,14 +1450,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;
>
> ...can we simple move this and similar (in other branches) to...
>
>
>> - } else {
>> - rdt_last_cmd_puts("Unknown or unsupported mode\n");
>> - ret = -EINVAL;
>> }
>
> ...here as
>
> rdtgrp->mode = user_m;
Will do.
It also looks like the only reason for the "mode" local variable was to
make those earlier "strcmp" lines shorter. With those long lines removed
in this patch this local variable is no longer needed and I will remove
it also.
Reinette
next prev parent reply other threads:[~2020-05-18 16:06 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-16 18:28 [PATCH V4 0/4] x86/resctrl: Enable user to view and select thread throttling mode Reinette Chatre
2020-05-16 18:28 ` [PATCH V4 1/4] " Reinette Chatre
2020-05-18 17:28 ` Babu Moger
2020-05-18 22:46 ` Reinette Chatre
2020-05-16 18:28 ` [PATCH V4 2/4] x86/resctrl: Enumerate per-thread MBA Reinette Chatre
2020-05-18 17:29 ` Babu Moger
2020-05-16 18:28 ` [PATCH V4 3/4] x86/resctrl: Enable " Reinette Chatre
2020-05-18 17:29 ` Babu Moger
2020-05-16 18:28 ` [PATCH V4 4/4] x86/resctrl: Use appropriate API for strings terminated by newline Reinette Chatre
2020-05-18 11:50 ` Andy Shevchenko
2020-05-18 16:06 ` Reinette Chatre [this message]
2020-05-18 16:23 ` Reinette Chatre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=060e3cf6-8fcb-6b43-01ef-63ef0a5ff64a@intel.com \
--to=reinette.chatre@intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=babu.moger@amd.com \
--cc=bp@alien8.de \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=kuo-lang.tseng@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=ravi.v.shankar@intel.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).