2011-02-08 11:07:49

by Marek Belisko

[permalink] [raw]
Subject: [PATCH RFC] staging: olpc_dcon: Remove _strtoul() function.

olpc_dcon driver use self invented _strtoul function
which make similar check like strict_strtoul just extend
for space checking at last string place. Normally access
to sys file looks echo 1024 > /sys/... so space could be considered
as error character and we could simplify code using just strict_strtoul
function instead self invented.

Signed-off-by: Marek Belisko <[email protected]>
---
drivers/staging/olpc_dcon/olpc_dcon.c | 66 ++++++++++++++------------------
1 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.c b/drivers/staging/olpc_dcon/olpc_dcon.c
index b19cd34..df1f664 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon.c
@@ -521,49 +521,35 @@ static ssize_t dcon_resumeline_show(struct device *dev,
return sprintf(buf, "%d\n", resumeline);
}

-static int _strtoul(const char *buf, int len, unsigned int *val)
-{
-
- char *endp;
- unsigned int output = strict_strtoul(buf, &endp, 0);
- int size = endp - buf;
-
- if (*endp && isspace(*endp))
- size++;
-
- if (size != len)
- return -EINVAL;
-
- *val = output;
- return 0;
-}
-
static ssize_t dcon_output_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
- int output;
- int rc = -EINVAL;
+ unsigned long output;
+ int ret;

- if (_strtoul(buf, count, &output))
+ ret = strict_strtoul(buf, 10, &output);
+ if (ret)
+ return ret;
+
+ if ((output != DCON_OUTPUT_COLOR) && (output != DCON_OUTPUT_MONO))
return -EINVAL;

- if (output == DCON_OUTPUT_COLOR || output == DCON_OUTPUT_MONO) {
- dcon_set_output(output);
- rc = count;
- }
+ dcon_set_output(output);

- return rc;
+ return count;
}

static ssize_t dcon_freeze_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
- int output;
+ unsigned long output;
+ int ret;

- if (_strtoul(buf, count, &output))
- return -EINVAL;
+ ret = strict_strtoul(buf, 10, &output);
+ if (ret)
+ return ret;

- printk(KERN_INFO "dcon_freeze_store: %d\n", output);
+ printk(KERN_INFO "dcon_freeze_store: %lu\n", output);

switch (output) {
case 0:
@@ -585,28 +571,34 @@ static ssize_t dcon_freeze_store(struct device *dev,
static ssize_t dcon_resumeline_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
- int rl;
- int rc = -EINVAL;
+ unsigned long rl;
+ int ret;

- if (_strtoul(buf, count, &rl))
- return rc;
+ ret = strict_strtoul(buf, 10, &rl);
+ if (ret)
+ return ret;

resumeline = rl;
dcon_write(DCON_REG_SCAN_INT, resumeline);
- rc = count;

- return rc;
+ return count;
}

static ssize_t dcon_sleep_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
- int output;
+ unsigned long output;
+ int ret;

- if (_strtoul(buf, count, &output))
+ ret = strict_strtoul(buf, 10, &output);
+ if (ret)
+ return ret;
+
+ if (output != 0 && output != 1)
return -EINVAL;

dcon_sleep(output ? DCON_SLEEP : DCON_ACTIVE);
+
return count;
}

--
1.7.1


2011-02-08 11:24:21

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH RFC] staging: olpc_dcon: Remove _strtoul() function.

On Tue, Feb 08, 2011 at 12:06:00PM +0100, Marek Belisko wrote:
> olpc_dcon driver use self invented _strtoul function
> which make similar check like strict_strtoul just extend
> for space checking at last string place. Normally access
> to sys file looks echo 1024 > /sys/... so space could be considered
> as error character and we could simplify code using just strict_strtoul
> function instead self invented.
>

Could you do it on top of the patches that Andres Salomon sent?

http://driverdev.linuxdriverproject.org/pipermail/devel/2011-February/thread.html

# [PATCH 1/4] olpc_dcon: get rid of global i2c_client, create a dcon_priv struct Andres Salomon
# [PATCH 2/4] olpc_dcon: change sysfs 'output' toggle to be clearer... Andres Salomon
# [PATCH 3/4] olpc_dcon: move more variables into dcon_priv Andres Salomon
# [PATCH 4/4] olpc_dcon: actually return the value of i2c_add_driver Andres Salomon

regards,
dan carpenter

2011-02-08 12:02:40

by Belisko Marek

[permalink] [raw]
Subject: Re: [PATCH RFC] staging: olpc_dcon: Remove _strtoul() function.

On Tue, Feb 8, 2011 at 12:23 PM, Dan Carpenter <[email protected]> wrote:
> On Tue, Feb 08, 2011 at 12:06:00PM +0100, Marek Belisko wrote:
>> olpc_dcon driver use self invented _strtoul  function
>> which make similar check like strict_strtoul just extend
>> for space checking at last string place. Normally access
>> to sys file looks echo 1024 > /sys/... so space could be considered
>> as error character and we could simplify code using just strict_strtoul
>> function instead self invented.
>>
>
> Could you do it on top of the patches that Andres Salomon sent?
patch resend : http://driverdev.linuxdriverproject.org/pipermail/devel/2011-February/011930.html
>
> http://driverdev.linuxdriverproject.org/pipermail/devel/2011-February/thread.html
>
> # [PATCH 1/4] olpc_dcon: get rid of global i2c_client, create a dcon_priv struct   Andres Salomon
> # [PATCH 2/4] olpc_dcon: change sysfs 'output' toggle to be clearer...   Andres Salomon
> # [PATCH 3/4] olpc_dcon: move more variables into dcon_priv   Andres Salomon
> # [PATCH 4/4] olpc_dcon: actually return the value of i2c_add_driver   Andres Salomon
>
> regards,
> dan carpenter
>
>
>

thanks,

marek

--
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
icq: 290551086
web: http://open-nandra.com

2011-02-08 17:40:26

by Andres Salomon

[permalink] [raw]
Subject: Re: [PATCH RFC] staging: olpc_dcon: Remove _strtoul() function.

On Tue, 8 Feb 2011 14:23:54 +0300
Dan Carpenter <[email protected]> wrote:

> On Tue, Feb 08, 2011 at 12:06:00PM +0100, Marek Belisko wrote:
> > olpc_dcon driver use self invented _strtoul function
> > which make similar check like strict_strtoul just extend
> > for space checking at last string place. Normally access
> > to sys file looks echo 1024 > /sys/... so space could be considered
> > as error character and we could simplify code using just
> > strict_strtoul function instead self invented.
> >
>
> Could you do it on top of the patches that Andres Salomon sent?
>

Actually, I have more pending patches. I can add this to the patchset
and send it later. Greg, please don't merge it now, though.

2011-02-09 20:12:54

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH RFC] staging: olpc_dcon: Remove _strtoul() function.

On Tue, Feb 08, 2011 at 09:40:20AM -0800, Andres Salomon wrote:
> On Tue, 8 Feb 2011 14:23:54 +0300
> Dan Carpenter <[email protected]> wrote:
>
> > On Tue, Feb 08, 2011 at 12:06:00PM +0100, Marek Belisko wrote:
> > > olpc_dcon driver use self invented _strtoul function
> > > which make similar check like strict_strtoul just extend
> > > for space checking at last string place. Normally access
> > > to sys file looks echo 1024 > /sys/... so space could be considered
> > > as error character and we could simplify code using just
> > > strict_strtoul function instead self invented.
> > >
> >
> > Could you do it on top of the patches that Andres Salomon sent?
> >
>
> Actually, I have more pending patches. I can add this to the patchset
> and send it later. Greg, please don't merge it now, though.

Ok, I've ignored it for now.