* [PATCH v2] ocfs2: heartbeat: replace simple_strtoul with kstrtoul
@ 2024-11-15 7:51 Daniel Yang
2024-11-17 12:37 ` Joseph Qi
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Yang @ 2024-11-15 7:51 UTC (permalink / raw)
To: Mark Fasheh, Joel Becker, Joseph Qi, GitAuthor: Daniel Yang,
open list:ORACLE CLUSTER FILESYSTEM 2 (OCFS2),
open list
The function simple_strtoul is deprecated due to ignoring overflows and
also requires clunkier error checking. Replacing with kstrtoul() leads
to safer code and cleaner error checking.
Signed-off-by: Daniel Yang <danielyangkang@gmail.com>
---
v1->v2: moved ret definition and removed blank lines
fs/ocfs2/cluster/heartbeat.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index dff18efbc..84ee5e340 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
@@ -1536,9 +1536,8 @@ static int o2hb_read_block_input(struct o2hb_region *reg,
{
unsigned long bytes;
char *p = (char *)page;
-
- bytes = simple_strtoul(p, &p, 0);
- if (!p || (*p && (*p != '\n')))
+ int ret = kstrtoul(p, 0, &bytes);
+ if (ret)
return -EINVAL;
/* Heartbeat and fs min / max block sizes are the same. */
@@ -1623,12 +1622,13 @@ static ssize_t o2hb_region_blocks_store(struct config_item *item,
struct o2hb_region *reg = to_o2hb_region(item);
unsigned long tmp;
char *p = (char *)page;
+ int ret;
if (reg->hr_bdev_file)
return -EINVAL;
- tmp = simple_strtoul(p, &p, 0);
- if (!p || (*p && (*p != '\n')))
+ ret = kstrtoul(p, 0, &tmp);
+ if (ret)
return -EINVAL;
if (tmp > O2NM_MAX_NODES || tmp == 0)
@@ -2141,9 +2141,8 @@ static ssize_t o2hb_heartbeat_group_dead_threshold_store(struct config_item *ite
{
unsigned long tmp;
char *p = (char *)page;
-
- tmp = simple_strtoul(p, &p, 10);
- if (!p || (*p && (*p != '\n')))
+ int ret = kstrtoul(p, 10, &tmp);
+ if (ret)
return -EINVAL;
/* this will validate ranges for us. */
--
2.39.5
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ocfs2: heartbeat: replace simple_strtoul with kstrtoul
2024-11-15 7:51 [PATCH v2] ocfs2: heartbeat: replace simple_strtoul with kstrtoul Daniel Yang
@ 2024-11-17 12:37 ` Joseph Qi
2024-11-17 21:53 ` Daniel Yang
0 siblings, 1 reply; 3+ messages in thread
From: Joseph Qi @ 2024-11-17 12:37 UTC (permalink / raw)
To: Daniel Yang
Cc: open list:ORACLE CLUSTER FILESYSTEM 2 (OCFS2),
open list, Mark Fasheh, Joel Becker
On 11/15/24 3:51 PM, Daniel Yang wrote:
> The function simple_strtoul is deprecated due to ignoring overflows and
> also requires clunkier error checking. Replacing with kstrtoul() leads
> to safer code and cleaner error checking.
>
> Signed-off-by: Daniel Yang <danielyangkang@gmail.com>
> ---
> v1->v2: moved ret definition and removed blank lines
>
> fs/ocfs2/cluster/heartbeat.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
> index dff18efbc..84ee5e340 100644
> --- a/fs/ocfs2/cluster/heartbeat.c
> +++ b/fs/ocfs2/cluster/heartbeat.c
> @@ -1536,9 +1536,8 @@ static int o2hb_read_block_input(struct o2hb_region *reg,
> {
> unsigned long bytes;
> char *p = (char *)page;
> -
> - bytes = simple_strtoul(p, &p, 0);
> - if (!p || (*p && (*p != '\n')))
> + int ret = kstrtoul(p, 0, &bytes);
> + if (ret)
The preferred code style is:
...
int ret;
ret = kstrtoul(p, 0, &bytes);
if (ret < 0)
return ret;
> return -EINVAL;
>
> /* Heartbeat and fs min / max block sizes are the same. */
> @@ -1623,12 +1622,13 @@ static ssize_t o2hb_region_blocks_store(struct config_item *item,
> struct o2hb_region *reg = to_o2hb_region(item);
> unsigned long tmp;
> char *p = (char *)page;
> + int ret;
>
> if (reg->hr_bdev_file)
> return -EINVAL;
>
> - tmp = simple_strtoul(p, &p, 0);
> - if (!p || (*p && (*p != '\n')))
> + ret = kstrtoul(p, 0, &tmp);
> + if (ret)
> return -EINVAL;
Better to return 'ret' directly since it may be -ERANGE.
>
> if (tmp > O2NM_MAX_NODES || tmp == 0)
> @@ -2141,9 +2141,8 @@ static ssize_t o2hb_heartbeat_group_dead_threshold_store(struct config_item *ite
> {
> unsigned long tmp;
> char *p = (char *)page;
> -
> - tmp = simple_strtoul(p, &p, 10);
> - if (!p || (*p && (*p != '\n')))
> + int ret = kstrtoul(p, 10, &tmp);
> + if (ret)
> return -EINVAL;
Prefer to:
...
int ret;
ret = kstrtoul(p, 10, &tmp);
if (ret < 0)
return ret;
Thanks,
Joseph
>
> /* this will validate ranges for us. */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ocfs2: heartbeat: replace simple_strtoul with kstrtoul
2024-11-17 12:37 ` Joseph Qi
@ 2024-11-17 21:53 ` Daniel Yang
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Yang @ 2024-11-17 21:53 UTC (permalink / raw)
To: Joseph Qi
Cc: open list:ORACLE CLUSTER FILESYSTEM 2 (OCFS2),
open list, Mark Fasheh, Joel Becker
On Sun, Nov 17, 2024 at 4:37 AM Joseph Qi <joseph.qi@linux.alibaba.com> wrote:
>
>
>
> On 11/15/24 3:51 PM, Daniel Yang wrote:
> > The function simple_strtoul is deprecated due to ignoring overflows and
> > also requires clunkier error checking. Replacing with kstrtoul() leads
> > to safer code and cleaner error checking.
> >
> > Signed-off-by: Daniel Yang <danielyangkang@gmail.com>
> > ---
> > v1->v2: moved ret definition and removed blank lines
> >
> > fs/ocfs2/cluster/heartbeat.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
> > index dff18efbc..84ee5e340 100644
> > --- a/fs/ocfs2/cluster/heartbeat.c
> > +++ b/fs/ocfs2/cluster/heartbeat.c
> > @@ -1536,9 +1536,8 @@ static int o2hb_read_block_input(struct o2hb_region *reg,
> > {
> > unsigned long bytes;
> > char *p = (char *)page;
> > -
> > - bytes = simple_strtoul(p, &p, 0);
> > - if (!p || (*p && (*p != '\n')))
> > + int ret = kstrtoul(p, 0, &bytes);
> > + if (ret)
>
> The preferred code style is:
>
> ...
> int ret;
>
> ret = kstrtoul(p, 0, &bytes);
> if (ret < 0)
> return ret;
>
> > return -EINVAL;
> >
> > /* Heartbeat and fs min / max block sizes are the same. */
> > @@ -1623,12 +1622,13 @@ static ssize_t o2hb_region_blocks_store(struct config_item *item,
> > struct o2hb_region *reg = to_o2hb_region(item);
> > unsigned long tmp;
> > char *p = (char *)page;
> > + int ret;
> >
> > if (reg->hr_bdev_file)
> > return -EINVAL;
> >
> > - tmp = simple_strtoul(p, &p, 0);
> > - if (!p || (*p && (*p != '\n')))
> > + ret = kstrtoul(p, 0, &tmp);
> > + if (ret)
> > return -EINVAL;
>
> Better to return 'ret' directly since it may be -ERANGE.
>
> >
> > if (tmp > O2NM_MAX_NODES || tmp == 0)
> > @@ -2141,9 +2141,8 @@ static ssize_t o2hb_heartbeat_group_dead_threshold_store(struct config_item *ite
> > {
> > unsigned long tmp;
> > char *p = (char *)page;
> > -
> > - tmp = simple_strtoul(p, &p, 10);
> > - if (!p || (*p && (*p != '\n')))
> > + int ret = kstrtoul(p, 10, &tmp);
> > + if (ret)
> > return -EINVAL;
>
> Prefer to:
>
> ...
> int ret;
>
> ret = kstrtoul(p, 10, &tmp);
> if (ret < 0)
> return ret;
>
> Thanks,
> Joseph
> >
> > /* this will validate ranges for us. */
>
Thanks for the clarification. I applied the changes and sent a v3 of the patch.
- Daniel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-17 21:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-15 7:51 [PATCH v2] ocfs2: heartbeat: replace simple_strtoul with kstrtoul Daniel Yang
2024-11-17 12:37 ` Joseph Qi
2024-11-17 21:53 ` Daniel Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®