From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <mg@max.gautier.name>
Received: from mout-p-101.mailbox.org (mout-p-101.mailbox.org
 [IPv6:2001:67c:2050::465:101])
 by sourceware.org (Postfix) with ESMTPS id A881E3858C27
 for <libc-alpha@sourceware.org>; Sat, 12 Mar 2022 11:08:02 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A881E3858C27
Received: from smtp1.mailbox.org (smtp1.mailbox.org [80.241.60.240])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest
 SHA256) (No client certificate requested)
 by mout-p-101.mailbox.org (Postfix) with ESMTPS id 4KG0QT1cYsz9sW2;
 Sat, 12 Mar 2022 12:08:01 +0100 (CET)
Date: Sat, 12 Mar 2022 12:07:57 +0100
From: Max Gautier <mg@max.gautier.name>
To: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Cc: libc-alpha@sourceware.org
Subject: Re: [PATCH v4 3/4] iconv: make utf-7.c able to use variants
Message-ID: <Yix/DQzmP9UWLBZ6@ol-mgautier>
Mail-Followup-To: Max Gautier <mg@max.gautier.name>,
 Adhemerval Zanella <adhemerval.zanella@linaro.org>,
 libc-alpha@sourceware.org
References: <87blcw9ptq.fsf@oldenburg.str.redhat.com>
 <20211209093152.313872-1-mg@max.gautier.name>
 <20211209093152.313872-4-mg@max.gautier.name>
 <1b82f293-bff9-f642-e836-38981023ec94@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1b82f293-bff9-f642-e836-38981023ec94@linaro.org>
X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00, DKIM_SIGNED,
 DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_PASS, TXREP,
 T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4
X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on
 server2.sourceware.org
X-BeenThere: libc-alpha@sourceware.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org>
List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>,
 <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe>
List-Archive: <https://sourceware.org/pipermail/libc-alpha/>
List-Post: <mailto:libc-alpha@sourceware.org>
List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help>
List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>,
 <mailto:libc-alpha-request@sourceware.org?subject=subscribe>
X-List-Received-Date: Sat, 12 Mar 2022 11:08:04 -0000

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