Re: [PATCH v4 3/4] iconv: make utf-7.c able to use variants

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

On Mon, Mar 07, 2022 at 09:34:47AM -0300, Adhemerval Zanella wrote:
> >  static int
> > -isxdirect (uint32_t ch)
> > +isxdirect (uint32_t ch, enum variant var)
> >  {
> > -    return (ch == '\t'
> > -            || ch == '\n'
> > -            || ch == '\r'
> > -            || (between(ch, ' ','}')
> > -                && ch != '+' && ch != '\\')
> > -           );
> > +    return(isdirect(ch, var)
> > +            || (var == UTF7 &&
> > +                (between(ch, '!', '&')
> > +                 || ch == '*'
> > +                 || between(ch, ';', '@')
> > +                 || (between(ch, '[', '`') && ch != '\\')
> > +                 || between(ch, '{', '}'))
> > +                )
> > +          );
> >  }
> >  
> 
> The change is ok, but maybe adding the variant out makes it more clear:
> 
>    if (var != UTF7)
>      return 0;
>    [...]
> 
something like this ? 

if (isdirect(ch, var))
   return true;
if (var != UTF7)
   return false;
return (between(ch, '!', '&')
  || ch == '*'
  || between(ch, ';', '@')
  || (between(ch, '[', '`') && ch != '\\')
  || between(ch, '{', '}'))
 );

I'd prefer the single expression form, but that works too.

> Also I think since you change it, it would be better to use 'bool' as
> return type.

Ok. I was not sure whether it was ok to use bool type or not.

Thanks for the review.

-- 
Max Gautier