* [patch] w1: small type cleanup in sysfs
@ 2014-02-11 16:08 Dan Carpenter
2014-02-11 16:45 ` David Fries
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2014-02-11 16:08 UTC (permalink / raw)
To: Evgeniy Polyakov, David Fries; +Cc: linux-kernel, kernel-janitors
On 64 bit systems, a large value for "tmp" could be truncated so we
still end up with a ->max_slave_count which is less than one despite the
"tmp < 1" check.
This is more of a problem for static checkers than a real life issue,
but it's simple enough to fix.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 9eb816b2ea5e..b96f61b15dc6 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -320,10 +320,10 @@ static ssize_t w1_master_attribute_show_timeout(struct device *dev, struct devic
static ssize_t w1_master_attribute_store_max_slave_count(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
- long tmp;
+ int tmp;
struct w1_master *md = dev_to_w1_master(dev);
- if (kstrtol(buf, 0, &tmp) = -EINVAL || tmp < 1)
+ if (kstrtoint(buf, 0, &tmp) = -EINVAL || tmp < 1)
return -EINVAL;
mutex_lock(&md->mutex);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] w1: small type cleanup in sysfs
2014-02-11 16:08 [patch] w1: small type cleanup in sysfs Dan Carpenter
@ 2014-02-11 16:45 ` David Fries
2014-02-11 21:07 ` [patch v2] " Dan Carpenter
2014-02-11 21:12 ` [patch] " Dan Carpenter
0 siblings, 2 replies; 5+ messages in thread
From: David Fries @ 2014-02-11 16:45 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Evgeniy Polyakov, linux-kernel, kernel-janitors
Dan,
I have some other changes in work, how automated is your checkers?
How much work is it for me to give a github repository and branch and
find out if I introduced any problems before submitting them?
I didn't get how you could get a less than one after a check for less
than one from the description or patch until I looked at the rest of
the source code. Looks good if the description mentions
max_slave_count is an int.
How about wording it,
On 64 bit systems, a large value for "long tmp" is truncated when
assigning to "int md->max_slave_count" so we still end up with a value
less than one despite the "tmp < 1" check.
Acked-by: David Fries <david@fries.net>
On Tue, Feb 11, 2014 at 07:08:26PM +0300, Dan Carpenter wrote:
> On 64 bit systems, a large value for "tmp" could be truncated so we
> still end up with a ->max_slave_count which is less than one despite the
> "tmp < 1" check.
>
> This is more of a problem for static checkers than a real life issue,
> but it's simple enough to fix.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index 9eb816b2ea5e..b96f61b15dc6 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -320,10 +320,10 @@ static ssize_t w1_master_attribute_show_timeout(struct device *dev, struct devic
> static ssize_t w1_master_attribute_store_max_slave_count(struct device *dev,
> struct device_attribute *attr, const char *buf, size_t count)
> {
> - long tmp;
> + int tmp;
> struct w1_master *md = dev_to_w1_master(dev);
>
> - if (kstrtol(buf, 0, &tmp) = -EINVAL || tmp < 1)
> + if (kstrtoint(buf, 0, &tmp) = -EINVAL || tmp < 1)
> return -EINVAL;
>
> mutex_lock(&md->mutex);
--
David Fries <david@fries.net> PGP pub CB1EE8F0
http://fries.net/~david/
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch v2] w1: small type cleanup in sysfs
2014-02-11 16:45 ` David Fries
@ 2014-02-11 21:07 ` Dan Carpenter
2014-02-14 15:42 ` Evgeniy Polyakov
2014-02-11 21:12 ` [patch] " Dan Carpenter
1 sibling, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2014-02-11 21:07 UTC (permalink / raw)
To: Evgeniy Polyakov, David Fries; +Cc: linux-kernel, kernel-janitors
On 64 bit systems, a large value for "long tmp" is truncated when
assigning to "int md->max_slave_count" so we still end up with a value
less than one despite the "tmp < 1" check.
This is more of a problem for static checkers than a real life issue,
but it's simple enough to fix.
Acked-by: David Fries <david@fries.net>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: clarified the commit message
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 9eb816b2ea5e..b96f61b15dc6 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -320,10 +320,10 @@ static ssize_t w1_master_attribute_show_timeout(struct device *dev, struct devic
static ssize_t w1_master_attribute_store_max_slave_count(struct device *dev,
struct device_attribute *attr, const char *buf, size_t count)
{
- long tmp;
+ int tmp;
struct w1_master *md = dev_to_w1_master(dev);
- if (kstrtol(buf, 0, &tmp) = -EINVAL || tmp < 1)
+ if (kstrtoint(buf, 0, &tmp) = -EINVAL || tmp < 1)
return -EINVAL;
mutex_lock(&md->mutex);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] w1: small type cleanup in sysfs
2014-02-11 16:45 ` David Fries
2014-02-11 21:07 ` [patch v2] " Dan Carpenter
@ 2014-02-11 21:12 ` Dan Carpenter
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2014-02-11 21:12 UTC (permalink / raw)
To: David Fries; +Cc: Evgeniy Polyakov, linux-kernel, kernel-janitors
On Tue, Feb 11, 2014 at 10:45:13AM -0600, David Fries wrote:
> Dan,
>
> I have some other changes in work, how automated is your checkers?
> How much work is it for me to give a github repository and branch and
> find out if I introduced any problems before submitting them?
>
This is a Smatch thing I'm working on but haven't pushed yet.
Eventually it will be a part of Fengguang Wu's automated kbuild zero
day testing process.
>
> I didn't get how you could get a less than one after a check for less
> than one from the description or patch until I looked at the rest of
> the source code. Looks good if the description mentions
> max_slave_count is an int.
>
> How about wording it,
>
> On 64 bit systems, a large value for "long tmp" is truncated when
> assigning to "int md->max_slave_count" so we still end up with a value
> less than one despite the "tmp < 1" check.
>
Sure. That's clearer. I have resent it.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch v2] w1: small type cleanup in sysfs
2014-02-11 21:07 ` [patch v2] " Dan Carpenter
@ 2014-02-14 15:42 ` Evgeniy Polyakov
0 siblings, 0 replies; 5+ messages in thread
From: Evgeniy Polyakov @ 2014-02-14 15:42 UTC (permalink / raw)
To: Dan Carpenter, David Fries; +Cc: linux-kernel, kernel-janitors
Hi
12.02.2014, 01:08, "Dan Carpenter" <dan.carpenter@oracle.com>:
> On 64 bit systems, a large value for "long tmp" is truncated when
> assigning to "int md->max_slave_count" so we still end up with a value
> less than one despite the "tmp < 1" check.
>
> This is more of a problem for static checkers than a real life issue,
> but it's simple enough to fix.
>
> Acked-by: David Fries <david@fries.net>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Looks good to me, thank you
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-14 15:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-11 16:08 [patch] w1: small type cleanup in sysfs Dan Carpenter
2014-02-11 16:45 ` David Fries
2014-02-11 21:07 ` [patch v2] " Dan Carpenter
2014-02-14 15:42 ` Evgeniy Polyakov
2014-02-11 21:12 ` [patch] " Dan Carpenter
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).