diff mbox series

[2/5] argp: Don't rely on undefined behaviour of _tolower().

Message ID 1609981580-17229-3-git-send-email-bruno@clisp.org
State Committed
Commit 1b3fc33f810b605e0e6dfcba96dddae432ccaab3
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 
  Patch by Eric Blake
<https://lists.gnu.org/archive/html/bug-gnulib/2009-09/msg00287.html>.

* argp/argp-help.c (hol_entry_cmp): Don't use _tolower on values that are
not upper-case.  Pass correct range to tolower.
---
 argp/argp-help.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Adhemerval Zanella Feb. 2, 2021, 2:33 p.m. UTC | #1 
On 06/01/2021 22:06, Bruno Haible wrote:
> Patch by Eric Blake
> <https://lists.gnu.org/archive/html/bug-gnulib/2009-09/msg00287.html>.
> 
> * argp/argp-help.c (hol_entry_cmp): Don't use _tolower on values that are
> not upper-case.  Pass correct range to tolower.

LGTM, thanks.

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

> ---
>  argp/argp-help.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/argp/argp-help.c b/argp/argp-help.c
> index f417e12..5844d5b 100644
> --- a/argp/argp-help.c
> +++ b/argp/argp-help.c
> @@ -780,13 +780,11 @@ hol_entry_cmp (const struct hol_entry *entry1,
>  	   first, but as they're not displayed, it doesn't matter where
>  	   they are.  */
>  	{
> -	  char first1 = short1 ? short1 : long1 ? *long1 : 0;
> -	  char first2 = short2 ? short2 : long2 ? *long2 : 0;
> -#ifdef _tolower
> -	  int lower_cmp = _tolower (first1) - _tolower (first2);
> -#else
> +	  unsigned char first1 = short1 ? short1 : long1 ? *long1 : 0;
> +	  unsigned char first2 = short2 ? short2 : long2 ? *long2 : 0;
> +	  /* Use tolower, not _tolower, since the latter has undefined
> +	     behaviour for characters that are not uppercase letters.  */
>  	  int lower_cmp = tolower (first1) - tolower (first2);
> -#endif
>  	  /* Compare ignoring case, except when the options are both the
>  	     same letter, in which case lower-case always comes first.  */
>  	  return lower_cmp ? lower_cmp : first2 - first1;
>
diff mbox series

Patch

diff --git a/argp/argp-help.c b/argp/argp-help.c
index f417e12..5844d5b 100644
--- a/argp/argp-help.c
+++ b/argp/argp-help.c
@@ -780,13 +780,11 @@  hol_entry_cmp (const struct hol_entry *entry1,
 	   first, but as they're not displayed, it doesn't matter where
 	   they are.  */
 	{
-	  char first1 = short1 ? short1 : long1 ? *long1 : 0;
-	  char first2 = short2 ? short2 : long2 ? *long2 : 0;
-#ifdef _tolower
-	  int lower_cmp = _tolower (first1) - _tolower (first2);
-#else
+	  unsigned char first1 = short1 ? short1 : long1 ? *long1 : 0;
+	  unsigned char first2 = short2 ? short2 : long2 ? *long2 : 0;
+	  /* Use tolower, not _tolower, since the latter has undefined
+	     behaviour for characters that are not uppercase letters.  */
 	  int lower_cmp = tolower (first1) - tolower (first2);
-#endif
 	  /* Compare ignoring case, except when the options are both the
 	     same letter, in which case lower-case always comes first.  */
 	  return lower_cmp ? lower_cmp : first2 - first1;