Re: gperf/src/options.cc -- quiesce clang warnings -Wlogical-op-parentheses

From: David Chisnall <theraven_at_FreeBSD.org>
Date: Thu, 24 Oct 2013 21:24:32 -0400
On 24 Oct 2013, at 21:13, Sean Bruno <sean_bruno_at_yahoo.com> wrote:

> On Tue, 2013-10-22 at 09:47 +0100, David Chisnall wrote:
>> On 22 Oct 2013, at 00:43, Sean Bruno <sean_bruno_at_yahoo.com> wrote:
>> 
>>> Heh, Matthew suggested the obvious in private mail, it seems that this
>>> would be better "spelled" as "isalpha" :-)
>> 
>> This looks wrong.  The behaviour of isalpha() depends on the current locale.  You probably want isalpha_l(), with the "C" locale.
>> 
>> David
> 
> Took me a bit of wrangling to figure out what the proper implementation
> of isalpha_l() and friends.
> 
> How about this then?
> 
> Index: options.cc
> ===================================================================
> --- options.cc	(revision 257083)
> +++ options.cc	(working copy)
> _at__at_ -28,6 +28,7 _at__at_
> #include <string.h> /* declares strcmp() */
> #include <ctype.h>  /* declares isdigit() */
> #include <limits.h> /* defines CHAR_MAX */
> +#include <xlocale.h>/* support for newlocale() */
> #include "getopt.h"
> #include "version.h"
> 
> _at__at_ -275,13 +276,15 _at__at_
>   for (int i = 0; i < _argument_count; i++)
>     {
>       const char *arg = _argument_vector[i];
> +      locale_t loc;	
> 
> +      loc = newlocale(LC_ALL_MASK, "C", 0);
>       /* Escape arg if it contains shell metacharacters.  */
>       if (*arg == '-')
>         {
>           putchar (*arg);
>           arg++;
> -          if (*arg >= 'A' && *arg <= 'Z' || *arg >= 'a' && *arg <= 'z')
> +          if (isalpha_l(*arg, loc))
>             {
>               putchar (*arg);
>               arg++;
> _at__at_ -293,7 +296,7 _at__at_
>                   putchar (*arg);
>                   arg++;
>                 }
> -              while (*arg >= 'A' && *arg <= 'Z' || *arg >= 'a' && *arg
> <= 'z' || *arg == '-');
> +              while (isalpha_l(*arg, loc) || *arg == '-');
>               if (*arg == '=')
>                 {
>                   putchar (*arg);

Don't forget the freelocale() at the end.

This seems like a very slow way of doing what was very fast in the original code though.  I'm not entirely sure what you're aiming to gain in this refactoring.

David
Received on Thu Oct 24 2013 - 23:24:41 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:43 UTC