From: Jeff Smith <whydoubt@gmail.com>
Subject: Re: [RFC PATCH 3/9] ntdll: Fix functions that depend on wctype table.
Message-Id: <CAPX7N=66X_sMNPBp30n=cBc-tD=3+4RZiv=cpPaxp5iFsT2u-A@mail.gmail.com>
Date: Thu, 9 Jan 2020 10:31:07 -0600
In-Reply-To: <adab6b25-2dbc-bb8b-e270-837413378c28@codeweavers.com>
References: <20200109065349.2549777-1-whydoubt@gmail.com> <20200109065349.2549777-3-whydoubt@gmail.com> <ed8165b8-b52b-9df6-ee87-f76532493ae7@codeweavers.com> <adab6b25-2dbc-bb8b-e270-837413378c28@codeweavers.com>

On Thu, Jan 9, 2020 at 9:14 AM Jacek Caban <jacek@codeweavers.com> wrote:
>
> On 09.01.2020 16:09, Jacek Caban wrote:
> > Hi Jeff,
> >
> > On 09.01.2020 07:53, Jeff Smith wrote:
> >> +/* Some abbreviations to make the following table readable */
> >> +#define_C_  C1_CNTRL
> >> +#define_S_  C1_SPACE
> >> +#define_P_  C1_PUNCT
> >> +#define_D_  C1_DIGIT
> >> +#define_H_  C1_XDIGIT
> >> +#define_U_  C1_UPPER|C1_ALPHA
> >> +#define_L_  C1_LOWER|C1_ALPHA
> >> +#define_B_  C1_BLANK
> >> +
> >> +static const WORD NTDLL_wctype [256] = {
> >> +_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_S_|_C_|_B_,_S_|_C_,_S_|_C_,
> >> +_S_|_C_,_S_|_C_,_C_,_C_,
> >> +_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,
> >> +_S_|_B_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,
> >> +_D_|_H_,_D_|_H_,_D_|_H_,_D_|_H_,_D_|_H_,_D_|_H_,_D_|_H_,_D_|_H_,_D_|_H_,_D_|_H_,
> >>
> >> +_P_,_P_,_P_,_P_,_P_,_P_,
> >> +_P_,_U_|_H_,_U_|_H_,_U_|_H_,_U_|_H_,_U_|_H_,_U_|_H_,_U_,_U_,_U_,_U_,_U_,
> >>
> >> +_U_,_U_,_U_,_U_,
> >> +_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_P_,_P_,_P_,_P_,_P_,
> >> +_P_,_L_|_H_,_L_|_H_,_L_|_H_,_L_|_H_,_L_|_H_,_L_|_H_,_L_,_L_,_L_,_L_,_L_,
> >>
> >> +_L_,_L_,_L_,_L_,
> >> +_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_P_,_P_,_P_,_P_,_C_,
> >> +_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,
> >> +_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,_C_,
> >> +_S_|_B_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,_P_,
> >> +_P_,_P_,_P_|_D_,_P_|_D_,_P_,_P_,_P_,_P_,_P_,_P_|_D_,_P_,_P_,_P_,_P_,_P_,_P_,
> >>
> >> +_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,
> >> +_U_,_U_,_U_,_U_,_U_,_U_,_U_,_P_,_U_,_U_,_U_,_U_,_U_,_U_,_U_,_L_,
> >> +_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,
> >> +_L_,_L_,_L_,_L_,_L_,_L_,_L_,_P_,_L_,_L_,_L_,_L_,_L_,_L_,_L_,_L_
> >> +};
> >
> >
> > Those macros don't really make the table much more readable, IMHO. You

Yeah, "readable" is certainly debatable.  The comment and table was
based on msvcrt/ctype.c which I filled out and reformatted to make
sure every 16th entry was on a new line (then wrapped a few that were
rather long).  I still found this form slightly easier to work with,
especially when it came to the creating the one for msvcrt/wcs.c.

> > could just use numeric literals and make it more compact. In fact, you
> > could probably just copy the first chunk of wctype_table from
> > kernelbase/wctype.c.

Well, it would need to be the first 32 lines starting after the
"value" comment, and it would need to be filtered "| 0x1ff", but it
would work.  I don't think that would be more compact though.

> Also, this patch needs a careful review of code that uses those
> function, most notably kernelbase.dll. A quick look suggests that it
> depends on working at least iswalpha for values above 255, so it will
> need to be changed first.

Agreed, that may be a bit of a chore, but still needed.

> Thanks,
>
> Jacek
>