lkml.org 
[lkml]   [2011]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 16/52] kstrtox: convert drivers/edac/
On Sat, Feb 05, 2011 at 06:34:49PM +0100, Borislav Petkov wrote:
> On Sat, Feb 05, 2011 at 04:20:19PM +0200, Alexey Dobriyan wrote:
>
> Some kind of a boilerplate commit message is still better than none at all.
>
> >
> > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> > ---
> > drivers/edac/amd64_edac_inj.c | 135 +++++++++++++++++------------------------
> > drivers/edac/edac_mc_sysfs.c | 20 +++---
> > drivers/edac/i7core_edac.c | 41 +++++++------
> > drivers/edac/mce_amd_inj.c | 28 ++++-----
> > 4 files changed, 101 insertions(+), 123 deletions(-)
> >
> > diff --git a/drivers/edac/amd64_edac_inj.c b/drivers/edac/amd64_edac_inj.c
> > index 688478d..180a498 100644
> > --- a/drivers/edac/amd64_edac_inj.c
> > +++ b/drivers/edac/amd64_edac_inj.c
> > @@ -16,21 +16,18 @@ static ssize_t amd64_inject_section_store(struct mem_ctl_info *mci,
> > const char *data, size_t count)
> > {
> > struct amd64_pvt *pvt = mci->pvt_info;
> > - unsigned long value;
> > - int ret = 0;
> > -
> > - ret = strict_strtoul(data, 10, &value);
> > - if (ret != -EINVAL) {
> > -
> > - if (value > 3) {
> > - amd64_warn("%s: invalid section 0x%lx\n", __func__, value);
> > - return -EINVAL;
> > - }
> > -
> > - pvt->injection.section = (u32) value;
> > - return count;
> > + u32 value;
> > + int ret;
> > +
> > + ret = kstrtou32(data, 10, &value);
> > + if (ret < 0)
> > + return ret;
>
> newline here

A what? Why?

> > @@ -107,29 +98,22 @@ static ssize_t amd64_inject_read_store(struct mem_ctl_info *mci,
> > const char *data, size_t count)
> > {
> > struct amd64_pvt *pvt = mci->pvt_info;
> > - unsigned long value;
> > u32 section, word_bits;
> > - int ret = 0;
> > -
> > - ret = strict_strtoul(data, 10, &value);
> > - if (ret != -EINVAL) {
>
> Dropping those would mean that the injection read (and write) below
> would happen on any value written.

Yes, since the value written is unused, and all data were already
written via other files.

> Let's keep them and enforce a write
> of '1' to be only valid injection trigger like the rest of the /sysfs
> interfaces:

Right now, any number will do, so in fact it'd be better to use kstrtoul().

> ret = kstrtou8(data, 10, &value);
> if (ret < 0)
> return ret;
>
> if (value != 1)
> return -EINVAL;
> > @@ -197,17 +174,15 @@ struct mcidev_sysfs_attribute amd64_inj_attrs[] = {
> > {
> > .attr = {
> > .name = "inject_write",
> > - .mode = (S_IRUGO | S_IWUSR)
> > + .mode = S_IWUSR,
>
> This change is not mentioned anywhere, why do we need it?

File is write-only. I'll mention this in changelog.

> > - .show = NULL,
> > .store = amd64_inject_write_store,
> > },
> > {
> > .attr = {
> > .name = "inject_read",
> > - .mode = (S_IRUGO | S_IWUSR)
> > + .mode = S_IWUSR,



\
 
 \ /
  Last update: 2011-02-06 19:55    [W:0.192 / U:0.816 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site