Re: [PATCH v5 2/4] iconv: Better mapping to RFC for UTF-7

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 21/03/2022 08:59, Adhemerval Zanella wrote:
> 
> 
> On 21/03/2022 08:53, Adhemerval Zanella wrote:
>>
>>
>> On 20/03/2022 13:41, Max Gautier via Libc-alpha wrote:
>>> - Direct use of characters instead of arcane arrays
>>> - isxbase64 is not the Modified BASE64 alphabet, but the characters who
>>>   needs to trigger an explicit shift back to US-ASCII. Make that clearer
>>>
>>> Signed-off-by: Max Gautier <mg@max.gautier.name>
>>
>>
>> LGTM, thanks.
>>
>> Reviewed-by: Adhemerval Zanellla  <adhemerval.zanella@linaro.org>
>>
>>> ---
>>>  iconvdata/utf-7.c | 64 ++++++++++++++++++++++++-----------------------
>>>  1 file changed, 33 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/iconvdata/utf-7.c b/iconvdata/utf-7.c
>>> index 9ba0974959..15f3669ac8 100644
>>> --- a/iconvdata/utf-7.c
>>> +++ b/iconvdata/utf-7.c
>>> @@ -30,20 +30,27 @@
>>>  
>>>  
>>>  
>>> +static bool
>>> +between (uint32_t const ch,
>>> +	 uint32_t const lower_bound, uint32_t const upper_bound)
>>> +{
>>> +  return (ch >= lower_bound && ch <= upper_bound);
>>> +}
>>> +
>>>  /* The set of "direct characters":
>>>     A-Z a-z 0-9 ' ( ) , - . / : ? space tab lf cr
>>>  */
>>>  
>>> -static const unsigned char direct_tab[128 / 8] =
>>> -  {
>>> -    0x00, 0x26, 0x00, 0x00, 0x81, 0xf3, 0xff, 0x87,
>>> -    0xfe, 0xff, 0xff, 0x07, 0xfe, 0xff, 0xff, 0x07
>>> -  };
>>> -
>>> -static int
>>> -isdirect (uint32_t ch)
>>> +static bool
>>> +isdirect (uint32_t ch, enum variant var)
>>>  {
> 
> In fact I am seeing this failure:
> 
> utf-7.c:45:29: error: ‘enum variant’ declared inside parameter list will not be visible outside of this definition o
> r declaration [-Werror]
>    45 | isdirect (uint32_t ch, enum variant var)
>       |                             ^~~~~~~
> 
> Since 'enum variant' in only defined on next patch.  Usually the best
> practice is keep each patch consistent, so could you move the definition
> on this patch?
> 
> Or I can fix it for you before installing, it is up to you.

And it does not actually required the variant argument on neither, isdirect or
isxdirect.  The obvious fix for this patch is:

diff --git a/iconvdata/utf-7.c b/iconvdata/utf-7.c
index 4a89de235a..815b1891c7 100644
--- a/iconvdata/utf-7.c
+++ b/iconvdata/utf-7.c
@@ -42,7 +42,7 @@ between (uint32_t const ch,
 */
 
 static bool
-isdirect (uint32_t ch, enum variant var)
+isdirect (uint32_t ch)
 {
   return (between (ch, 'A', 'Z')
          || between (ch, 'a', 'z')
@@ -60,7 +60,7 @@ isdirect (uint32_t ch, enum variant var)
 */
 
 static bool
-isxdirect (uint32_t ch, enum variant var)
+isxdirect (uint32_t ch)
 {
   return (ch == '\t'
          || ch == '\n'