On Thu, Jul 8, 2010 at 8:04 PM, Ira Weiny <wei...@llnl.gov> wrote:
>
> On Thu, 8 Jul 2010 10:37:26 -0700
> Bart Van Assche <bvanass...@acm.org> wrote:
>
> > On Thu, Jul 8, 2010 at 2:33 AM, Ira Weiny <wei...@llnl.gov> wrote:
> > > From 80eecc4046455999254fb312c4ba229b3a52d4c6 Mon Sep 17 00:00:00 2001
> > > From: Ira Weiny <wei...@llnl.gov>
> > > Date: Wed, 7 Jul 2010 17:35:34 -0700
> > > Subject: [PATCH] ib_qib: Allow writes to the diag_counters to be able to 
> > > clear them
> > >
> > >
> > > Signed-off-by: Ira Weiny <wei...@llnl.gov>
> > > ---
> > >  drivers/infiniband/hw/qib/qib_sysfs.c |   16 +++++++++++++++-
> > >  1 files changed, 15 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/qib/qib_sysfs.c 
> > > b/drivers/infiniband/hw/qib/qib_sysfs.c
> > > index dab4d9f..91cd1b8 100644
> > > --- a/drivers/infiniband/hw/qib/qib_sysfs.c
> > > +++ b/drivers/infiniband/hw/qib/qib_sysfs.c
> > > @@ -347,7 +347,7 @@ static struct kobj_type qib_sl2vl_ktype = {
> > >
> > >  #define QIB_DIAGC_ATTR(N) \
> > >        static struct qib_diagc_attr qib_diagc_attr_##N = { \
> > > -               .attr = { .name = __stringify(N), .mode = 0444 }, \
> > > +               .attr = { .name = __stringify(N), .mode = 0664 }, \
> > >                .counter = offsetof(struct qib_ibport, n_##N) \
> > >        }
> > >
> > > @@ -403,8 +403,22 @@ static ssize_t diagc_attr_show(struct kobject *kobj, 
> > > struct attribute *attr,
> > >        return sprintf(buf, "%u\n", *(u32 *)((char *)qibp + 
> > > dattr->counter));
> > >  }
> > >
> > > +static ssize_t diagc_attr_store(struct kobject *kobj, struct attribute 
> > > *attr,
> > > +                               const char *buf, size_t size)
> > > +{
> > > +       struct qib_diagc_attr *dattr =
> > > +               container_of(attr, struct qib_diagc_attr, attr);
> > > +       struct qib_pportdata *ppd =
> > > +               container_of(kobj, struct qib_pportdata, diagc_kobj);
> > > +       struct qib_ibport *qibp = &ppd->ibport_data;
> > > +
> > > +       *(u32 *)((char *)qibp + dattr->counter) = simple_strtol(buf, 
> > > NULL, 0);
> > > +       return 4;
> >
> > The above line is not correct -- it should probably be something like
> > "return size;". See also
> > http://*git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/filesystems/sysfs.txt.
>
> My mistake.  From the document above the return should be:
>
> return strnlen(buf, PAGE_SIZE);
>
> Correct?
>
> Also I think I should check for invalid values as well.

(resending as plain text)

The documented I referred to was written before the "size" argument
was added to sysfs store methods. I'm not sure it is still required
that the "buf" argument that is passed to store methods is
'\0'-terminated. So both "return 4" and "return strnlen(buf,
PAGE_SIZE)" can potentially return a value that is larger than the
"size" argument, which I think is incorrect.

Sorry that I pointed you to misleading documentation.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to