diff mbox series

[libubootenv,v2,2/2] validate_flags: Use isdigit() instead of strtoull()

Message ID 20230615130854.2700871-2-pro@denx.de
State Accepted
Delegated to: Stefano Babic
Headers show
Series [libubootenv,v2,1/2] validate_flags: Bugfix for TYPE_ATTR_HEX | expand

Commit Message

Philip Oberfichtner June 15, 2023, 1:08 p.m. UTC 
Using strtoull() is unreliable for testing if the input is a number. It
is implementation-defined, whether or not errno is set.

Furthermore, the decimal type has to be handled separately from the hex
type. U-Boot will reject an "0x10" entry as decimal, "16" is expected
instead.

Reviewed-by: Stefano Babic <sbabic@denx.de>
Signed-off-by: Philip Oberfichtner <pro@denx.de>
---

Changes in v2: Reviewed by Stefano

 src/uboot_env.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

Comments

Stefano Babic June 15, 2023, 1:14 p.m. UTC | #1 
Hi Philip.

On 15.06.23 15:08, Philip Oberfichtner wrote:
> Using strtoull() is unreliable for testing if the input is a number. It
> is implementation-defined, whether or not errno is set.
> 
> Furthermore, the decimal type has to be handled separately from the hex
> type. U-Boot will reject an "0x10" entry as decimal, "16" is expected
> instead.
> 
> Reviewed-by: Stefano Babic <sbabic@denx.de>
> Signed-off-by: Philip Oberfichtner <pro@denx.de>
> ---
> 
> Changes in v2: Reviewed by Stefano
> 

You do not need it. This is managed automatically by Patchwork. You can 
test this by getting the previous version (pwclient get 1795271), you 
will see that Reviewed-by was automatically added.

Best regards,
Stefano

>   src/uboot_env.c | 26 +++++++++++++++++++-------
>   1 file changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/src/uboot_env.c b/src/uboot_env.c
> index a31e459..b1e5923 100644
> --- a/src/uboot_env.c
> +++ b/src/uboot_env.c
> @@ -1681,10 +1681,24 @@ int libuboot_read_config(struct uboot_ctx *ctx, const char *config)
>   	return libuboot_read_config_ext(&ctx, config);
>   }
>   
> +static bool validate_int(bool hex, const char *value)
> +{
> +	const char *c;
> +
> +	for (c = value; c != value + strlen(value); ++c) {
> +		if (hex && !isxdigit(*c))
> +			return false;
> +
> +		if (!hex && !isdigit(*c))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>   static bool libuboot_validate_flags(struct var_entry *entry, const char *value)
>   {
>   	bool ok_type = true, ok_access = true;
> -	unsigned long long test;
>   
>   	switch (entry->access) {
>   	case ACCESS_ATTR_ANY:
> @@ -1709,15 +1723,13 @@ static bool libuboot_validate_flags(struct var_entry *entry, const char *value)
>   		ok_type = true;
>   		break;
>   	case TYPE_ATTR_DECIMAL:
> +		ok_type = validate_int(false, value);
> +		break;
>   	case TYPE_ATTR_HEX:
> -		errno = 0;
>   		ok_type = strlen(value) > 2 && (value[0] == '0') &&
>   			(value[1] == 'x' || value [1] == 'X');
> -		if (ok_type) {
> -			test = strtoull(value, NULL, 16);
> -			if (errno)
> -				ok_type = false;
> -		}
> +		if (ok_type)
> +			ok_type = validate_int(true, value + 2);
>   		break;
>   	case TYPE_ATTR_BOOL:
>   		ok_access = (value[0] == '1' || value[0] == 'y' || value[0] == 't' ||
diff mbox series

Patch

diff --git a/src/uboot_env.c b/src/uboot_env.c
index a31e459..b1e5923 100644
--- a/src/uboot_env.c
+++ b/src/uboot_env.c
@@ -1681,10 +1681,24 @@  int libuboot_read_config(struct uboot_ctx *ctx, const char *config)
 	return libuboot_read_config_ext(&ctx, config);
 }
 
+static bool validate_int(bool hex, const char *value)
+{
+	const char *c;
+
+	for (c = value; c != value + strlen(value); ++c) {
+		if (hex && !isxdigit(*c))
+			return false;
+
+		if (!hex && !isdigit(*c))
+			return false;
+	}
+
+	return true;
+}
+
 static bool libuboot_validate_flags(struct var_entry *entry, const char *value)
 {
 	bool ok_type = true, ok_access = true;
-	unsigned long long test;
 
 	switch (entry->access) {
 	case ACCESS_ATTR_ANY:
@@ -1709,15 +1723,13 @@  static bool libuboot_validate_flags(struct var_entry *entry, const char *value)
 		ok_type = true;
 		break;
 	case TYPE_ATTR_DECIMAL:
+		ok_type = validate_int(false, value);
+		break;
 	case TYPE_ATTR_HEX:
-		errno = 0;
 		ok_type = strlen(value) > 2 && (value[0] == '0') &&
 			(value[1] == 'x' || value [1] == 'X');
-		if (ok_type) {
-			test = strtoull(value, NULL, 16);
-			if (errno)
-				ok_type = false;
-		}
+		if (ok_type)
+			ok_type = validate_int(true, value + 2);
 		break;
 	case TYPE_ATTR_BOOL:
 		ok_access = (value[0] == '1' || value[0] == 'y' || value[0] == 't' ||