On Fri, 2008-03-28 at 14:10 -0700, NeilBrown wrote: > On Sat, March 29, 2008 6:33 am, Dan Williams wrote: > > raid5_store_preread_threshold(mddev_t *mddev, const char *page, size_t > > len) > > { > > raid5_conf_t *conf = mddev_to_conf(mddev); > > - char *end; > > - int new; > > + unsigned long new; > > if (len >= PAGE_SIZE) > > return -EINVAL; > > if (!conf) > > return -ENODEV; > > > > - new = simple_strtoul(page, &end, 10); > > - if (!*page || (*end && *end != '\n')) > > + if (strict_strtoul(page, 10, &new)) > > return -EINVAL; > > - if (new > conf->max_nr_stripes || new < 0) > > + if (new > conf->max_nr_stripes || (int) new < 0) > > I had suggested that "new < 0" test when I saw that 'new' was an 'int'. > A better suggestion would have been to make 'new' 'unsigned'. > Now that you have done that, the "< 0" it pointless and > should go. Yeah, ->max_nr_stripes will always be less than (1 << 63). > Otherwise > Acked-By: NeilBrown <neilb@xxxxxxx> Thanks, new patch follows, Dan ------snip------> Subject: md: raid5.c convert simple_strtoul to strict_strtoul From: Dan Williams <dan.j.williams@xxxxxxxxx> strict_strtoul handles the open-coded sanity checks in raid5_store_stripe_cache_size and raid5_store_preread_threshold Acked-by: NeilBrown <neilb@xxxxxxx> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx> --- drivers/md/raid5.c | 14 +++++--------- 1 files changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index bc39369..e43e27a 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -4034,15 +4034,13 @@ static ssize_t raid5_store_stripe_cache_size(mddev_t *mddev, const char *page, size_t len) { raid5_conf_t *conf = mddev_to_conf(mddev); - char *end; - int new; + unsigned long new; if (len >= PAGE_SIZE) return -EINVAL; if (!conf) return -ENODEV; - new = simple_strtoul(page, &end, 10); - if (!*page || (*end && *end != '\n') ) + if (strict_strtoul(page, 10, &new)) return -EINVAL; if (new <= 16 || new > 32768) return -EINVAL; @@ -4080,17 +4078,15 @@ static ssize_t raid5_store_preread_threshold(mddev_t *mddev, const char *page, size_t len) { raid5_conf_t *conf = mddev_to_conf(mddev); - char *end; - int new; + unsigned long new; if (len >= PAGE_SIZE) return -EINVAL; if (!conf) return -ENODEV; - new = simple_strtoul(page, &end, 10); - if (!*page || (*end && *end != '\n')) + if (strict_strtoul(page, 10, &new)) return -EINVAL; - if (new > conf->max_nr_stripes || new < 0) + if (new > conf->max_nr_stripes) return -EINVAL; conf->bypass_threshold = new; return len; -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html