[PATCH] ndctl: Fix the NDCTL_TIMEOUT environment variable parsing

[PATCH] ndctl: Fix the NDCTL_TIMEOUT environment variable parsing

[Shivaprasad G Bhat]

The strtoul(x, y, size) returns empty string on y when the x is "only"
number with no other suffix strings. The code is checking if !null
instead of comparing with empty string.

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
---
 ndctl/lib/libndctl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ad54f06..b0287e8 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -334,7 +334,7 @@ NDCTL_EXPORT int ndctl_new(struct ndctl_ctx **ctx)
 		char *end;
 
 		tmo = strtoul(env, &end, 0);
-		if (tmo < ULONG_MAX && !end)
+		if (tmo < ULONG_MAX && strcmp(end, "") == 0)
 			c->timeout = tmo;
 		dbg(c, "timeout = %ld\n", tmo);
 	}

Re: [PATCH] ndctl: Fix the NDCTL_TIMEOUT environment variable parsing

[Vaibhav Jain]

Hi Shiva,

Thanks for fixing this. Minor review comment below:

Shivaprasad G Bhat <sbhat@linux.ibm.com> writes:

> The strtoul(x, y, size) returns empty string on y when the x is "only"
> number with no other suffix strings. The code is checking if !null
> instead of comparing with empty string.
>
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>
> ---
>  ndctl/lib/libndctl.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
> index ad54f06..b0287e8 100644
> --- a/ndctl/lib/libndctl.c
> +++ b/ndctl/lib/libndctl.c
> @@ -334,7 +334,7 @@ NDCTL_EXPORT int ndctl_new(struct ndctl_ctx **ctx)
>  		char *end;
>  
>  		tmo = strtoul(env, &end, 0);
> -		if (tmo < ULONG_MAX && !end)
> +		if (tmo < ULONG_MAX && strcmp(end, "") == 0)

Using strcmp would be better avoided in new code. Instead you can check
for the valid string to parse in strtoull() with simply checking against
*end == '\0' or !*end.

Quote for STRTOUL(3):

"if *nptr is not '\0' but **endptr is '\0' on return,  the  entire string
is valid."


>  			c->timeout = tmo;
>  		dbg(c, "timeout = %ld\n", tmo);
>  	}
>
>
>
>

-- 
Cheers
~ Vaibhav