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

[Max Gautier <mg@max.gautier.name>]

On Mon, Mar 21, 2022 at 08:59:27AM -0300, 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.
> 

I think I mixed up my patches while integrating the corrections and
style fixes you mentionned, sorry.
No problem for me I you fix it before applying.

Thanks !

-- 
Max Gautier