Home Original page

[PATCH v2 31/65] MIPS: use is_whitespace()

Jan Beulich jbeulich@suse.com
Mon Feb 3 15:52:15 GMT 2025 
On 03.02.2025 15:07, Maciej W. Rozycki wrote:
> On Mon, 27 Jan 2025, Jan Beulich wrote:
> 
>> --- a/gas/config/tc-mips.c
>> +++ b/gas/config/tc-mips.c
>> @@ -14388,7 +14388,7 @@ mips16_ip (char *str, struct mips_cl_ins
>>    struct mips_operand_token *tokens;
>>    unsigned int l;
>>  
>> -  for (s = str; *s != '\0' && *s != '.' && *s != ' '; ++s)
>> +  for (s = str; *s != '\0' && *s != '.' && !is_whitespace (*s); ++s)
>                    ^^^^^^^^^^
>  Shouldn't this also be `!is_end_of_stmt (*s)'?

This is the kind of question you as the target maintainer really
will want to answer. All I could do is try to guess where
conversion might be on order.

>> @@ -14399,8 +14399,9 @@ mips16_ip (char *str, struct mips_cl_ins
>>      case '\0':
>>        break;
>>  
>> -    case ' ':
>> -      s++;
>> +    default:
>> +      if (is_whitespace (*s))
>> +	s++;
>>        break;
> 
>  Why `is_whitespace (*s)' rather than `is_whitespace (c)'?

Right, I should have noticed to use c here.

>  I think this only causes obfuscation to this already messed up statement.  
> Since there are only two cases here really ('\0' does nothing and is the 
> only remaining possibility here, guaranteed by the loop right above) can 
> you please rewrite this as:
> 
>   if (c == '.')
>     {
>       ...
>     }
>   else if (is_whitespace (c))
>     s++;
> 
> or suchlike?

Possibly. On a similar question from Richard on aarch64 I indicated that
from other projects I'm working on I'm used to using switch() in such
cases, even if at a certain point there may be just a single case label.
This is to ease future addition of new further labels.

>> @@ -14417,7 +14418,7 @@ mips16_ip (char *str, struct mips_cl_ins
>>  	}
>>        if (*s == '\0')
> 
>  And `is_end_of_stmt (*s)' here presumably too?

See above. It has been a lot of targets all doing things (often just
slightly) differently, so I can only guess that I may have got the
impression that somewhere up the callstack something nul-terminates the
string.

Jan
More information about the Binutils mailing list