[3/5] argp: Don't pass invalid arguments to isspace, isalnum, isalpha, isdigit.

Message ID 1609981580-17229-4-git-send-email-bruno@clisp.org
State Committed
Commit e9f63b512621fec9fc794719506dd306f3eaa39d
Delegated to: Adhemerval Zanella Netto
Headers
Series argp: Fix several cases of undefined behaviour |

Commit Message

Bruno Haible Jan. 7, 2021, 1:06 a.m. UTC
  * lib/argp-help.c (SKIPWS): Cast character to 'unsigned char' before passing it
to isspace().
(fill_in_uparams): Likewise for isalpha(), isalnum(), isdigit().
(canon_doc_option): Likewise for isspace(), isalnum().
---
 argp/argp-help.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
  

Comments

Adhemerval Zanella Netto Feb. 2, 2021, 2:35 p.m. UTC | #1
On 06/01/2021 22:06, Bruno Haible wrote:
> * lib/argp-help.c (SKIPWS): Cast character to 'unsigned char' before passing it
> to isspace().
> (fill_in_uparams): Likewise for isalpha(), isalnum(), isdigit().
> (canon_doc_option): Likewise for isspace(), isalnum().

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  argp/argp-help.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/argp/argp-help.c b/argp/argp-help.c
> index 5844d5b..d686a23 100644
> --- a/argp/argp-help.c
> +++ b/argp/argp-help.c
> @@ -166,7 +166,7 @@ fill_in_uparams (const struct argp_state *state)
>  {
>    const char *var = getenv ("ARGP_HELP_FMT");
>  
> -#define SKIPWS(p) do { while (isspace (*p)) p++; } while (0);
> +#define SKIPWS(p) do { while (isspace ((unsigned char) *p)) p++; } while (0);
>  
>    if (var)
>      /* Parse var. */
> @@ -174,14 +174,14 @@ fill_in_uparams (const struct argp_state *state)
>        {
>  	SKIPWS (var);
>  
> -	if (isalpha (*var))
> +	if (isalpha ((unsigned char) *var))
>  	  {
>  	    size_t var_len;
>  	    const struct uparam_name *un;
>  	    int unspec = 0, val = 0;
>  	    const char *arg = var;
>  
> -	    while (isalnum (*arg) || *arg == '-' || *arg == '_')
> +	    while (isalnum ((unsigned char) *arg) || *arg == '-' || *arg == '_')
>  	      arg++;
>  	    var_len = arg - var;
>  
> @@ -206,10 +206,10 @@ fill_in_uparams (const struct argp_state *state)
>  		else
>  		  val = 1;
>  	      }
> -	    else if (isdigit (*arg))
> +	    else if (isdigit ((unsigned char) *arg))
>  	      {
>  		val = atoi (arg);
> -		while (isdigit (*arg))
> +		while (isdigit ((unsigned char) *arg))
>  		  arg++;
>  		SKIPWS (arg);
>  	      }
> @@ -713,12 +713,12 @@ canon_doc_option (const char **name)
>  {
>    int non_opt;
>    /* Skip initial whitespace.  */
> -  while (isspace (**name))
> +  while (isspace ((unsigned char) **name))
>      (*name)++;
>    /* Decide whether this looks like an option (leading `-') or not.  */
>    non_opt = (**name != '-');
>    /* Skip until part of name used for sorting.  */
> -  while (**name && !isalnum (**name))
> +  while (**name && !isalnum ((unsigned char) **name))
>      (*name)++;
>    return non_opt;
>  }
>
  

Patch

diff --git a/argp/argp-help.c b/argp/argp-help.c
index 5844d5b..d686a23 100644
--- a/argp/argp-help.c
+++ b/argp/argp-help.c
@@ -166,7 +166,7 @@  fill_in_uparams (const struct argp_state *state)
 {
   const char *var = getenv ("ARGP_HELP_FMT");
 
-#define SKIPWS(p) do { while (isspace (*p)) p++; } while (0);
+#define SKIPWS(p) do { while (isspace ((unsigned char) *p)) p++; } while (0);
 
   if (var)
     /* Parse var. */
@@ -174,14 +174,14 @@  fill_in_uparams (const struct argp_state *state)
       {
 	SKIPWS (var);
 
-	if (isalpha (*var))
+	if (isalpha ((unsigned char) *var))
 	  {
 	    size_t var_len;
 	    const struct uparam_name *un;
 	    int unspec = 0, val = 0;
 	    const char *arg = var;
 
-	    while (isalnum (*arg) || *arg == '-' || *arg == '_')
+	    while (isalnum ((unsigned char) *arg) || *arg == '-' || *arg == '_')
 	      arg++;
 	    var_len = arg - var;
 
@@ -206,10 +206,10 @@  fill_in_uparams (const struct argp_state *state)
 		else
 		  val = 1;
 	      }
-	    else if (isdigit (*arg))
+	    else if (isdigit ((unsigned char) *arg))
 	      {
 		val = atoi (arg);
-		while (isdigit (*arg))
+		while (isdigit ((unsigned char) *arg))
 		  arg++;
 		SKIPWS (arg);
 	      }
@@ -713,12 +713,12 @@  canon_doc_option (const char **name)
 {
   int non_opt;
   /* Skip initial whitespace.  */
-  while (isspace (**name))
+  while (isspace ((unsigned char) **name))
     (*name)++;
   /* Decide whether this looks like an option (leading `-') or not.  */
   non_opt = (**name != '-');
   /* Skip until part of name used for sorting.  */
-  while (**name && !isalnum (**name))
+  while (**name && !isalnum ((unsigned char) **name))
     (*name)++;
   return non_opt;
 }