* [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint()
[not found] <1400575582-24841-1-git-send-email-zhenzhang.zhang@huawei.com>
@ 2014-05-20 8:48 ` Zhang Zhen
2014-05-20 12:21 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Zhang Zhen @ 2014-05-20 8:48 UTC (permalink / raw)
To: Geert Uytterhoeven, adrian.hunter, Artem Bityutskiy
Cc: MTD Maling List, Hu Jianyang
use the newer and more pleasant kstrtoint() to replace simple_strtoul(),
because simple_strtoul() is marked for obsoletion.
Signed-off-by: Zhang Zhen <zhenzhang.zhang@huawei.com>
Signed-off-by: hujianyang <hujianyang@huawei.com>
---
drivers/mtd/ubi/build.c | 7 +++----
fs/ubifs/super.c | 7 ++++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 6e30a3c..77a2884 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -1190,11 +1190,10 @@ static struct mtd_info * __init open_mtd_by_chdev(const char *mtd_dev)
static struct mtd_info * __init open_mtd_device(const char *mtd_dev)
{
struct mtd_info *mtd;
- int mtd_num;
- char *endp;
+ int mtd_num, ret;
- mtd_num = simple_strtoul(mtd_dev, &endp, 0);
- if (*endp != '\0' || mtd_dev == endp) {
+ ret = kstrtoint(mtd_dev, 0, &mtd_num);
+ if (ret) {
/*
* This does not look like an ASCII integer, probably this is
* MTD device name.
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index a81c7b5..d8c71a6 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1903,7 +1903,7 @@ const struct super_operations ubifs_super_operations = {
static struct ubi_volume_desc *open_ubi(const char *name, int mode)
{
struct ubi_volume_desc *ubi;
- int dev, vol;
+ int dev, vol, ret;
char *endptr;
/* First, try to open using the device node path method */
@@ -1922,10 +1922,11 @@ static struct ubi_volume_desc *open_ubi(const char *name, int mode)
if (!isdigit(name[3]))
return ERR_PTR(-EINVAL);
- dev = simple_strtoul(name + 3, &endptr, 0);
+ endptr = (char *)name + 3;
+ ret = kstrtoint(endptr, 0, &dev);
/* ubiY method */
- if (*endptr == '\0')
+ if (!ret)
return ubi_open_volume(0, dev, mode);
/* ubiX_Y method */
--
1.8.1.2
.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint()
2014-05-20 8:48 ` [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint() Zhang Zhen
@ 2014-05-20 12:21 ` Adrian Hunter
2014-05-20 12:31 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2014-05-20 12:21 UTC (permalink / raw)
To: Zhang Zhen, Geert Uytterhoeven, Artem Bityutskiy
Cc: MTD Maling List, Hu Jianyang
On 05/20/2014 11:48 AM, Zhang Zhen wrote:
> use the newer and more pleasant kstrtoint() to replace simple_strtoul(),
> because simple_strtoul() is marked for obsoletion.
>
> Signed-off-by: Zhang Zhen <zhenzhang.zhang@huawei.com>
> Signed-off-by: hujianyang <hujianyang@huawei.com>
> ---
> drivers/mtd/ubi/build.c | 7 +++----
> fs/ubifs/super.c | 7 ++++---
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 6e30a3c..77a2884 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1190,11 +1190,10 @@ static struct mtd_info * __init open_mtd_by_chdev(const char *mtd_dev)
> static struct mtd_info * __init open_mtd_device(const char *mtd_dev)
> {
> struct mtd_info *mtd;
> - int mtd_num;
> - char *endp;
> + int mtd_num, ret;
>
> - mtd_num = simple_strtoul(mtd_dev, &endp, 0);
> - if (*endp != '\0' || mtd_dev == endp) {
> + ret = kstrtoint(mtd_dev, 0, &mtd_num);
> + if (ret) {
> /*
> * This does not look like an ASCII integer, probably this is
> * MTD device name.
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index a81c7b5..d8c71a6 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -1903,7 +1903,7 @@ const struct super_operations ubifs_super_operations = {
> static struct ubi_volume_desc *open_ubi(const char *name, int mode)
> {
> struct ubi_volume_desc *ubi;
> - int dev, vol;
> + int dev, vol, ret;
> char *endptr;
>
> /* First, try to open using the device node path method */
> @@ -1922,10 +1922,11 @@ static struct ubi_volume_desc *open_ubi(const char *name, int mode)
> if (!isdigit(name[3]))
> return ERR_PTR(-EINVAL);
>
> - dev = simple_strtoul(name + 3, &endptr, 0);
> + endptr = (char *)name + 3;
> + ret = kstrtoint(endptr, 0, &dev);
But endptr is used in the code later, so this is wrong.
>
> /* ubiY method */
> - if (*endptr == '\0')
> + if (!ret)
> return ubi_open_volume(0, dev, mode);
>
> /* ubiX_Y method */
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint()
2014-05-20 12:21 ` Adrian Hunter
@ 2014-05-20 12:31 ` Geert Uytterhoeven
2014-05-20 12:45 ` Adrian Hunter
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-05-20 12:31 UTC (permalink / raw)
To: Adrian Hunter; +Cc: Hu Jianyang, MTD Maling List, Zhang Zhen, Artem Bityutskiy
On Tue, May 20, 2014 at 2:21 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 05/20/2014 11:48 AM, Zhang Zhen wrote:
>> --- a/fs/ubifs/super.c
>> +++ b/fs/ubifs/super.c
>> @@ -1903,7 +1903,7 @@ const struct super_operations ubifs_super_operations = {
>> static struct ubi_volume_desc *open_ubi(const char *name, int mode)
>> {
>> struct ubi_volume_desc *ubi;
>> - int dev, vol;
>> + int dev, vol, ret;
>> char *endptr;
>>
>> /* First, try to open using the device node path method */
>> @@ -1922,10 +1922,11 @@ static struct ubi_volume_desc *open_ubi(const char *name, int mode)
>> if (!isdigit(name[3]))
>> return ERR_PTR(-EINVAL);
>>
>> - dev = simple_strtoul(name + 3, &endptr, 0);
>> + endptr = (char *)name + 3;
Please make endptr const char *, so the cast can be killed.
Always think twice before adding a cast!
Ah, there's still another simple_strtoul() left below, that needs to be
converted first.
>> + ret = kstrtoint(endptr, 0, &dev);
>
> But endptr is used in the code later, so this is wrong.
That was my first thought, too. But upon closer look, I think it's correct.
>>
>> /* ubiY method */
>> - if (*endptr == '\0')
endptr would point to the trailing nul on success...
>> + if (!ret)
... which is now replaced by a test for ret not being an errorcoe.
>> return ubi_open_volume(0, dev, mode);
>>
>> /* ubiX_Y method */
If ret is an errorcode, the flow continues.
As parsing the number failed, the code checks if the first character
(name + 3) is an underscore or colon:
if (*endptr == '_' && isdigit(endptr[1])) {
vol = simple_strtoul(endptr + 1, &endptr, 0);
if (*endptr != '\0')
return ERR_PTR(-EINVAL);
return ubi_open_volume(dev, vol, mode);
}
/* ubiX:NAME method */
if ((*endptr == ':' || *endptr == '!') && endptr[1] != '\0')
return ubi_open_volume_nm(dev, ++endptr, mode);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint()
2014-05-20 12:31 ` Geert Uytterhoeven
@ 2014-05-20 12:45 ` Adrian Hunter
2014-05-20 12:55 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2014-05-20 12:45 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Hu Jianyang, MTD Maling List, Zhang Zhen, Artem Bityutskiy
On 05/20/2014 03:31 PM, Geert Uytterhoeven wrote:
> On Tue, May 20, 2014 at 2:21 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 05/20/2014 11:48 AM, Zhang Zhen wrote:
>>> --- a/fs/ubifs/super.c
>>> +++ b/fs/ubifs/super.c
>>> @@ -1903,7 +1903,7 @@ const struct super_operations ubifs_super_operations = {
>>> static struct ubi_volume_desc *open_ubi(const char *name, int mode)
>>> {
>>> struct ubi_volume_desc *ubi;
>>> - int dev, vol;
>>> + int dev, vol, ret;
>>> char *endptr;
>>>
>>> /* First, try to open using the device node path method */
>>> @@ -1922,10 +1922,11 @@ static struct ubi_volume_desc *open_ubi(const char *name, int mode)
>>> if (!isdigit(name[3]))
>>> return ERR_PTR(-EINVAL);
>>>
>>> - dev = simple_strtoul(name + 3, &endptr, 0);
>>> + endptr = (char *)name + 3;
>
> Please make endptr const char *, so the cast can be killed.
> Always think twice before adding a cast!
>
> Ah, there's still another simple_strtoul() left below, that needs to be
> converted first.
>
>>> + ret = kstrtoint(endptr, 0, &dev);
>>
>> But endptr is used in the code later, so this is wrong.
>
> That was my first thought, too. But upon closer look, I think it's correct.
>
>>>
>>> /* ubiY method */
>>> - if (*endptr == '\0')
>
> endptr would point to the trailing nul on success...
>
>>> + if (!ret)
>
> ... which is now replaced by a test for ret not being an errorcoe.
>
>>> return ubi_open_volume(0, dev, mode);
>>>
>>> /* ubiX_Y method */
>
> If ret is an errorcode, the flow continues.
>
> As parsing the number failed, the code checks if the first character
> (name + 3) is an underscore or colon:
No, it does not check if parsing failed, it checks for end-of-input.
The "X" and "Y" of "ubiX_Y" are numbers.
>
> if (*endptr == '_' && isdigit(endptr[1])) {
>
>
> vol = simple_strtoul(endptr + 1, &endptr, 0);
> if (*endptr != '\0')
> return ERR_PTR(-EINVAL);
> return ubi_open_volume(dev, vol, mode);
> }
>
> /* ubiX:NAME method */
> if ((*endptr == ':' || *endptr == '!') && endptr[1] != '\0')
> return ubi_open_volume_nm(dev, ++endptr, mode);
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint()
2014-05-20 12:45 ` Adrian Hunter
@ 2014-05-20 12:55 ` Geert Uytterhoeven
2014-05-21 2:22 ` Zhang Zhen
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2014-05-20 12:55 UTC (permalink / raw)
To: Adrian Hunter; +Cc: Hu Jianyang, MTD Maling List, Zhang Zhen, Artem Bityutskiy
On Tue, May 20, 2014 at 2:45 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> + ret = kstrtoint(endptr, 0, &dev);
>>>
>>> But endptr is used in the code later, so this is wrong.
>>
>> That was my first thought, too. But upon closer look, I think it's correct.
>>
>>>>
>>>> /* ubiY method */
>>>> - if (*endptr == '\0')
>>
>> endptr would point to the trailing nul on success...
>>
>>>> + if (!ret)
>>
>> ... which is now replaced by a test for ret not being an errorcoe.
>>
>>>> return ubi_open_volume(0, dev, mode);
>>>>
>>>> /* ubiX_Y method */
>>
>> If ret is an errorcode, the flow continues.
>>
>> As parsing the number failed, the code checks if the first character
>> (name + 3) is an underscore or colon:
>
> No, it does not check if parsing failed, it checks for end-of-input.
> The "X" and "Y" of "ubiX_Y" are numbers.
Sorry, you're right. Thanks for noticing!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint()
2014-05-20 12:55 ` Geert Uytterhoeven
@ 2014-05-21 2:22 ` Zhang Zhen
0 siblings, 0 replies; 6+ messages in thread
From: Zhang Zhen @ 2014-05-21 2:22 UTC (permalink / raw)
To: Adrian Hunter
Cc: MTD Maling List, Hu Jianyang, Geert Uytterhoeven,
Artem Bityutskiy
On 2014/5/20 20:55, Geert Uytterhoeven wrote:
> On Tue, May 20, 2014 at 2:45 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> + ret = kstrtoint(endptr, 0, &dev);
>>>>
>>>> But endptr is used in the code later, so this is wrong.
>>>
>>> That was my first thought, too. But upon closer look, I think it's correct.
>>>
>>>>>
>>>>> /* ubiY method */
>>>>> - if (*endptr == '\0')
>>>
>>> endptr would point to the trailing nul on success...
>>>
>>>>> + if (!ret)
>>>
>>> ... which is now replaced by a test for ret not being an errorcoe.
>>>
>>>>> return ubi_open_volume(0, dev, mode);
>>>>>
>>>>> /* ubiX_Y method */
>>>
>>> If ret is an errorcode, the flow continues.
>>>
>>> As parsing the number failed, the code checks if the first character
>>> (name + 3) is an underscore or colon:
>>
>> No, it does not check if parsing failed, it checks for end-of-input.
>> The "X" and "Y" of "ubiX_Y" are numbers.
Sorry, i made a mistake. As parsing the number failed, it should just return
ERR_PTR(-EINVAL).
Thank you for your review. I learned a lot from you and Geert.
I will send another version.
>
> Sorry, you're right. Thanks for noticing!
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-21 2:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1400575582-24841-1-git-send-email-zhenzhang.zhang@huawei.com>
2014-05-20 8:48 ` [PATCH v2] UBIFS: replace simple_strtoul() with kstrtoint() Zhang Zhen
2014-05-20 12:21 ` Adrian Hunter
2014-05-20 12:31 ` Geert Uytterhoeven
2014-05-20 12:45 ` Adrian Hunter
2014-05-20 12:55 ` Geert Uytterhoeven
2014-05-21 2:22 ` Zhang Zhen
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).