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

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