| Submitter | Lukas Czerner |
|---|---|
| Date | March 8, 2013, 8:23 a.m. |
| Message ID | <1362730998-3263-2-git-send-email-lczerner@redhat.com> |
| Download | mbox | patch |
| Permalink | /patch/226038/ |
| State | Superseded |
| Headers | show |
Comments
On Fri, Mar 08, 2013 at 09:23:17AM +0100, Lukas Czerner wrote: > In parse_strtoul() we're still using deprecated simple_strtoul(). Change > that in favour of kstrtoul(). > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> Once you make this change, wouldn't it be better and even more simplifying to replace the two places where we call parse_strtoul() with kstrtoul()? - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 10 Mar 2013, Theodore Ts'o wrote: > Date: Sun, 10 Mar 2013 22:26:35 -0400 > From: Theodore Ts'o <tytso@mit.edu> > To: Lukas Czerner <lczerner@redhat.com> > Cc: linux-ext4@vger.kernel.org > Subject: Re: [PATCH 2/3] ext4: Use kstrtoul() instead of deprecated > simple_strtoul() > > On Fri, Mar 08, 2013 at 09:23:17AM +0100, Lukas Czerner wrote: > > In parse_strtoul() we're still using deprecated simple_strtoul(). Change > > that in favour of kstrtoul(). > > > > Signed-off-by: Lukas Czerner <lczerner@redhat.com> > > Once you make this change, wouldn't it be better and even more > simplifying to replace the two places where we call parse_strtoul() > with kstrtoul()? > > - Ted I think that parse_strtoul() is still useful to have because we check the "max" value as well and return -EINVAL if it is exceeded. Removing it we would have to add the check to the callers where we're using is now, which seems unnecessary, especially since we might expect more users of the helper. -Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 11, 2013 at 08:36:16AM +0100, Lukáš Czerner wrote: > I think that parse_strtoul() is still useful to have because we > check the "max" value as well and return -EINVAL if it is exceeded. > Removing it we would have to add the check to the callers > where we're using is now, which seems unnecessary, especially since > we might expect more users of the helper. Well, at the moment only one of the two users of parse_strtoul() is using the "max" value check feature. I see one other potential user of parse_strtoul (there's one use of simple_strtoul in get_sb_block() which wasn't converted in your patch), but it wouldn't need the max value check feature either. So I don't have a super strong feeling about this, since this isn't performance critical code, and it's probably not going to cost a large amount of object code or stack space, but sometimes extra levels of abstraction end up hurting more than they help. Regards, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 620cf56..7bcdef1 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -2359,14 +2359,12 @@ struct ext4_attr { static int parse_strtoul(const char *buf, unsigned long max, unsigned long *value) { - char *endp; - - *value = simple_strtoul(skip_spaces(buf), &endp, 0); - endp = skip_spaces(endp); - if (*endp || *value > max) - return -EINVAL; + int ret; - return 0; + ret = kstrtoul(skip_spaces(buf), 0, value); + if (!ret && *value > max) + ret = -EINVAL; + return ret; } static ssize_t delayed_allocation_blocks_show(struct ext4_attr *a,
In parse_strtoul() we're still using deprecated simple_strtoul(). Change that in favour of kstrtoul(). Signed-off-by: Lukas Czerner <lczerner@redhat.com> --- fs/ext4/super.c | 12 +++++------- 1 files changed, 5 insertions(+), 7 deletions(-)