Re: [PATCH 1/1] fs/xfs remove obsolete simple_strto
On 01/09/2013 10:04 PM, Abhijit Pawar wrote:
> This patch replaces usages of obsolete simple_strtoul with kstrtoint in
> xfs_args and suffix_strtoul.
>
> Signed-off-by: Abhijit Pawar
> ---
> fs/xfs/xfs_super.c | 29 +++--
> 1 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ab8839b..c407121 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -139,9 +139,9 @@ static const match_table_t tokens = {
>
>
> STATIC unsigned long
> -suffix_strtoul(char *s, char **endp, unsigned int base)
> +suffix_kstrtoint(char *s, unsigned int base, int *res)
> {
> - int last, shift_left_factor = 0;
> + int last, shift_left_factor = 0, _res;
> char*value = s;
>
> last = strlen(value) - 1;
> @@ -158,7 +158,10 @@ suffix_strtoul(char *s, char **endp, unsigned int base)
> value[last] = '\0';
> }
>
> - return simple_strtoul((const char *)s, endp, base) << shift_left_factor;
> + if (kstrtoint(s, base, &_res))
> + return -EINVAL;
> + *res = _res << shift_left_factor;
> + return 0;
> }
>
> /*
> @@ -174,7 +177,7 @@ xfs_parseargs(
> char*options)
> {
> struct super_block *sb = mp->m_super;
> - char*this_char, *value, *eov;
> + char*this_char, *value;
> int dsunit = 0;
> int dswidth = 0;
> int iosize = 0;
> @@ -230,14 +233,16 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - mp->m_logbufs = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, >m_logbufs))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_LOGBSIZE)) {
> if (!value || !*value) {
> xfs_warn(mp, "%s option requires an argument",
> this_char);
> return EINVAL;
> }
> - mp->m_logbsize = suffix_strtoul(value, , 10);
> + if (suffix_kstrtoint(value, 10, >m_logbsize))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_LOGDEV)) {
> if (!value || !*value) {
> xfs_warn(mp, "%s option requires an argument",
> @@ -266,7 +271,8 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - iosize = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, ))
> + return EINVAL;
> iosizelog = ffs(iosize) - 1;
> } else if (!strcmp(this_char, MNTOPT_ALLOCSIZE)) {
> if (!value || !*value) {
> @@ -274,7 +280,8 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - iosize = suffix_strtoul(value, , 10);
> + if (suffix_kstrtoint(value, 10, ))
> + return EINVAL;
> iosizelog = ffs(iosize) - 1;
> } else if (!strcmp(this_char, MNTOPT_GRPID) ||
> !strcmp(this_char, MNTOPT_BSDGROUPS)) {
> @@ -296,14 +303,16 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - dsunit = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, ))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_SWIDTH)) {
> if (!value || !*value) {
> xfs_warn(mp, "%s option requires an argument",
> this_char);
> return EINVAL;
> }
> - dswidth = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, ))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
> mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> } else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
>
Reviewed-by: Jie Liu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] fs/xfs remove obsolete simple_strto
On 01/12/2013 06:52 AM, Dave Chinner wrote:
> On Fri, Jan 11, 2013 at 02:36:46PM +0800, Jeff Liu wrote:
>> On 01/09/2013 10:04 PM, Abhijit Pawar wrote:
>>> This patch replaces usages of obsolete simple_strtoul with kstrtoint in
>>> xfs_args and suffix_strtoul.
>>>
>>> Signed-off-by: Abhijit Pawar
>>> ---
>>> + if (kstrtoint(value, 10, ))
>>> + return EINVAL;
>>> } else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
>>> mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
>>> } else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
>>>
>> checkpatch.pl show warning if we return EINVAL as below:
>> WARNING: return of an errno should typically be -ve (return -EINVAL)
>>
>> Can we just ignore such code style issue?
>
> Returning a positive error is not a code style issue. It's a
> correctness issue. the core of the XFS code returns positive error
> numbers as that's the way it was done on Irix (where the XFs code
> comes from). The rest of the Linux code tends to use negative values
> for error returns, and we've never converted the XFS code base to
> negative errors.
>
> You should always feel free to ignore checkpatch warnings that make
> no sense. I haven't used checkpatch now for several years - I
> stopped using it when it got too noisy warning about uselesss,
> trivial things in the XFS code base
Thanks for the clarification, that would save me time to handle
checkpatch warnings against XFS in the future. :)
Cheers,
-Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] fs/xfs remove obsolete simple_strto
On Fri, Jan 11, 2013 at 02:36:46PM +0800, Jeff Liu wrote:
> On 01/09/2013 10:04 PM, Abhijit Pawar wrote:
> > This patch replaces usages of obsolete simple_strtoul with kstrtoint in
> > xfs_args and suffix_strtoul.
> >
> > Signed-off-by: Abhijit Pawar
> > ---
> > + if (kstrtoint(value, 10, ))
> > + return EINVAL;
> > } else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
> > mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> > } else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
> >
> checkpatch.pl show warning if we return EINVAL as below:
> WARNING: return of an errno should typically be -ve (return -EINVAL)
>
> Can we just ignore such code style issue?
Returning a positive error is not a code style issue. It's a
correctness issue. the core of the XFS code returns positive error
numbers as that's the way it was done on Irix (where the XFs code
comes from). The rest of the Linux code tends to use negative values
for error returns, and we've never converted the XFS code base to
negative errors.
You should always feel free to ignore checkpatch warnings that make
no sense. I haven't used checkpatch now for several years - I
stopped using it when it got too noisy warning about uselesss,
trivial things in the XFS code base
Cheers,
Dave.
--
Dave Chinner
da...@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] fs/xfs remove obsolete simple_strto
Hey, On Fri, Jan 11, 2013 at 01:16:30PM +0530, Abhijit Pawar wrote: > On 01/11/2013 12:06 PM, Jeff Liu wrote: > > On 01/09/2013 10:04 PM, Abhijit Pawar wrote: > >> This patch replaces usages of obsolete simple_strtoul with kstrtoint in > >> xfs_args and suffix_strtoul. > >> > >> Signed-off-by: Abhijit Pawar > > checkpatch.pl show warning if we return EINVAL as below: > > WARNING: return of an errno should typically be -ve (return -EINVAL) > > > > Can we just ignore such code style issue? > I think we can. > Ben? Yep, no worries. Regards, Ben -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] fs/xfs remove obsolete simple_strto
On 01/11/2013 12:06 PM, Jeff Liu wrote:
> On 01/09/2013 10:04 PM, Abhijit Pawar wrote:
>> This patch replaces usages of obsolete simple_strtoul with kstrtoint in
>> xfs_args and suffix_strtoul.
>>
>> Signed-off-by: Abhijit Pawar
>> ---
>> fs/xfs/xfs_super.c | 29 +++--
>> 1 files changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>> index ab8839b..c407121 100644
>> --- a/fs/xfs/xfs_super.c
>> +++ b/fs/xfs/xfs_super.c
>> @@ -139,9 +139,9 @@ static const match_table_t tokens = {
>>
>>
>> STATIC unsigned long
>> -suffix_strtoul(char *s, char **endp, unsigned int base)
>> +suffix_kstrtoint(char *s, unsigned int base, int *res)
>> {
>> -int last, shift_left_factor = 0;
>> +int last, shift_left_factor = 0, _res;
>> char*value = s;
>>
>> last = strlen(value) - 1;
>> @@ -158,7 +158,10 @@ suffix_strtoul(char *s, char **endp, unsigned int base)
>> value[last] = '\0';
>> }
>>
>> -return simple_strtoul((const char *)s, endp, base) << shift_left_factor;
>> +if (kstrtoint(s, base, &_res))
>> +return -EINVAL;
>> +*res = _res << shift_left_factor;
>> +return 0;
>> }
>>
>> /*
>> @@ -174,7 +177,7 @@ xfs_parseargs(
>> char*options)
>> {
>> struct super_block *sb = mp->m_super;
>> -char*this_char, *value, *eov;
>> +char*this_char, *value;
>> int dsunit = 0;
>> int dswidth = 0;
>> int iosize = 0;
>> @@ -230,14 +233,16 @@ xfs_parseargs(
>> this_char);
>> return EINVAL;
>> }
>> -mp->m_logbufs = simple_strtoul(value, , 10);
>> +if (kstrtoint(value, 10, >m_logbufs))
>> +return EINVAL;
>> } else if (!strcmp(this_char, MNTOPT_LOGBSIZE)) {
>> if (!value || !*value) {
>> xfs_warn(mp, "%s option requires an argument",
>> this_char);
>> return EINVAL;
>> }
>> -mp->m_logbsize = suffix_strtoul(value, , 10);
>> +if (suffix_kstrtoint(value, 10, >m_logbsize))
>> +return EINVAL;
>> } else if (!strcmp(this_char, MNTOPT_LOGDEV)) {
>> if (!value || !*value) {
>> xfs_warn(mp, "%s option requires an argument",
>> @@ -266,7 +271,8 @@ xfs_parseargs(
>> this_char);
>> return EINVAL;
>> }
>> -iosize = simple_strtoul(value, , 10);
>> +if (kstrtoint(value, 10, ))
>> +return EINVAL;
>> iosizelog = ffs(iosize) - 1;
>> } else if (!strcmp(this_char, MNTOPT_ALLOCSIZE)) {
>> if (!value || !*value) {
>> @@ -274,7 +280,8 @@ xfs_parseargs(
>> this_char);
>> return EINVAL;
>> }
>> -iosize = suffix_strtoul(value, , 10);
>> +if (suffix_kstrtoint(value, 10, ))
>> +return EINVAL;
>> iosizelog = ffs(iosize) - 1;
>> } else if (!strcmp(this_char, MNTOPT_GRPID) ||
>> !strcmp(this_char, MNTOPT_BSDGROUPS)) {
>> @@ -296,14 +303,16 @@ xfs_parseargs(
>> this_char);
>> return EINVAL;
>> }
>> -dsunit = simple_strtoul(value, , 10);
>> +if (kstrtoint(value, 10, ))
>> +return EINVAL;
>> } else if (!strcmp(this_char, MNTOPT_SWIDTH)) {
>> if (!value || !*value) {
>> xfs_warn(mp, "%s option requires an argument",
>> this_char);
>> return EINVAL;
>> }
>> -dswidth = simple_strtoul(value, , 10);
>> +if (kstrtoint(value, 10, ))
>> +return EINVAL;
>> } else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
>> mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
>> } else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
>>
> checkpatch.pl show warning if we return EINVAL as below:
> WARNING: return of an errno should typically be -ve (return -EINVAL)
>
> Can we just ignore such code style issue?
I think we can.
Ben?
>
> Thanks,
> -Jeff
>
--
-
Abhijit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Re: [PATCH 1/1] fs/xfs remove obsolete simple_strto
On 01/09/2013 10:04 PM, Abhijit Pawar wrote:
> This patch replaces usages of obsolete simple_strtoul with kstrtoint in
> xfs_args and suffix_strtoul.
>
> Signed-off-by: Abhijit Pawar
> ---
> fs/xfs/xfs_super.c | 29 +++--
> 1 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ab8839b..c407121 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -139,9 +139,9 @@ static const match_table_t tokens = {
>
>
> STATIC unsigned long
> -suffix_strtoul(char *s, char **endp, unsigned int base)
> +suffix_kstrtoint(char *s, unsigned int base, int *res)
> {
> - int last, shift_left_factor = 0;
> + int last, shift_left_factor = 0, _res;
> char*value = s;
>
> last = strlen(value) - 1;
> @@ -158,7 +158,10 @@ suffix_strtoul(char *s, char **endp, unsigned int base)
> value[last] = '\0';
> }
>
> - return simple_strtoul((const char *)s, endp, base) << shift_left_factor;
> + if (kstrtoint(s, base, &_res))
> + return -EINVAL;
> + *res = _res << shift_left_factor;
> + return 0;
> }
>
> /*
> @@ -174,7 +177,7 @@ xfs_parseargs(
> char*options)
> {
> struct super_block *sb = mp->m_super;
> - char*this_char, *value, *eov;
> + char*this_char, *value;
> int dsunit = 0;
> int dswidth = 0;
> int iosize = 0;
> @@ -230,14 +233,16 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - mp->m_logbufs = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, >m_logbufs))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_LOGBSIZE)) {
> if (!value || !*value) {
> xfs_warn(mp, "%s option requires an argument",
> this_char);
> return EINVAL;
> }
> - mp->m_logbsize = suffix_strtoul(value, , 10);
> + if (suffix_kstrtoint(value, 10, >m_logbsize))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_LOGDEV)) {
> if (!value || !*value) {
> xfs_warn(mp, "%s option requires an argument",
> @@ -266,7 +271,8 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - iosize = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, ))
> + return EINVAL;
> iosizelog = ffs(iosize) - 1;
> } else if (!strcmp(this_char, MNTOPT_ALLOCSIZE)) {
> if (!value || !*value) {
> @@ -274,7 +280,8 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - iosize = suffix_strtoul(value, , 10);
> + if (suffix_kstrtoint(value, 10, ))
> + return EINVAL;
> iosizelog = ffs(iosize) - 1;
> } else if (!strcmp(this_char, MNTOPT_GRPID) ||
> !strcmp(this_char, MNTOPT_BSDGROUPS)) {
> @@ -296,14 +303,16 @@ xfs_parseargs(
> this_char);
> return EINVAL;
> }
> - dsunit = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, ))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_SWIDTH)) {
> if (!value || !*value) {
> xfs_warn(mp, "%s option requires an argument",
> this_char);
> return EINVAL;
> }
> - dswidth = simple_strtoul(value, , 10);
> + if (kstrtoint(value, 10, ))
> + return EINVAL;
> } else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
> mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> } else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
>
checkpatch.pl show warning if we return EINVAL as below:
WARNING: return of an errno should typically be -ve (return -EINVAL)
Can we just ignore such code style issue?
Thanks,
-Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please
[PATCH 1/1] fs/xfs remove obsolete simple_strto
This patch replaces usages of obsolete simple_strtoul with kstrtoint in
xfs_args and suffix_strtoul.
Signed-off-by: Abhijit Pawar
---
fs/xfs/xfs_super.c | 29 +++--
1 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index ab8839b..c407121 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -139,9 +139,9 @@ static const match_table_t tokens = {
STATIC unsigned long
-suffix_strtoul(char *s, char **endp, unsigned int base)
+suffix_kstrtoint(char *s, unsigned int base, int *res)
{
- int last, shift_left_factor = 0;
+ int last, shift_left_factor = 0, _res;
char*value = s;
last = strlen(value) - 1;
@@ -158,7 +158,10 @@ suffix_strtoul(char *s, char **endp, unsigned int base)
value[last] = '\0';
}
- return simple_strtoul((const char *)s, endp, base) << shift_left_factor;
+ if (kstrtoint(s, base, &_res))
+ return -EINVAL;
+ *res = _res << shift_left_factor;
+ return 0;
}
/*
@@ -174,7 +177,7 @@ xfs_parseargs(
char*options)
{
struct super_block *sb = mp->m_super;
- char*this_char, *value, *eov;
+ char*this_char, *value;
int dsunit = 0;
int dswidth = 0;
int iosize = 0;
@@ -230,14 +233,16 @@ xfs_parseargs(
this_char);
return EINVAL;
}
- mp->m_logbufs = simple_strtoul(value, , 10);
+ if (kstrtoint(value, 10, >m_logbufs))
+ return EINVAL;
} else if (!strcmp(this_char, MNTOPT_LOGBSIZE)) {
if (!value || !*value) {
xfs_warn(mp, "%s option requires an argument",
this_char);
return EINVAL;
}
- mp->m_logbsize = suffix_strtoul(value, , 10);
+ if (suffix_kstrtoint(value, 10, >m_logbsize))
+ return EINVAL;
} else if (!strcmp(this_char, MNTOPT_LOGDEV)) {
if (!value || !*value) {
xfs_warn(mp, "%s option requires an argument",
@@ -266,7 +271,8 @@ xfs_parseargs(
this_char);
return EINVAL;
}
- iosize = simple_strtoul(value, , 10);
+ if (kstrtoint(value, 10, ))
+ return EINVAL;
iosizelog = ffs(iosize) - 1;
} else if (!strcmp(this_char, MNTOPT_ALLOCSIZE)) {
if (!value || !*value) {
@@ -274,7 +280,8 @@ xfs_parseargs(
this_char);
return EINVAL;
}
- iosize = suffix_strtoul(value, , 10);
+ if (suffix_kstrtoint(value, 10, ))
+ return EINVAL;
iosizelog = ffs(iosize) - 1;
} else if (!strcmp(this_char, MNTOPT_GRPID) ||
!strcmp(this_char, MNTOPT_BSDGROUPS)) {
@@ -296,14 +303,16 @@ xfs_parseargs(
this_char);
return EINVAL;
}
- dsunit = simple_strtoul(value, , 10);
+ if (kstrtoint(value, 10, ))
+ return EINVAL;
} else if (!strcmp(this_char, MNTOPT_SWIDTH)) {
if (!value || !*value) {
xfs_warn(mp, "%s option requires an argument",
this_char);
return EINVAL;
}
- dswidth = simple_strtoul(value, , 10);
+ if (kstrtoint(value, 10, ))
+ return EINVAL;
} else if (!strcmp(this_char, MNTOPT_32BITINODE)) {
mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
} else if (!strcmp(this_char, MNTOPT_64BITINODE)) {
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
