Message ID | CAGQ9bdz9V7w+5Wn_j8CSKvweh9GkW0gU5GvB94UCP2Zn-bf8OQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
Any comments on this patch? --kcc On Wed, Jun 18, 2014 at 5:59 PM, Konstantin Serebryany <konstantin.s.serebryany@gmail.com> wrote: > Hi, > > Please review the patch which removes nested functions findidx and replaces them > with two 'static inline' functions: findidx and findidxwc. > No functionality change intended. > > un-nesting glibc function was previously discussed here: > https://sourceware.org/ml/libc-alpha/2014-05/msg00400.html > > Thanks! > > --kcc > > 2014-06-18 Kostya Serebryany <konstantin.s.serebryany@gmail.com> > > * locale/weight.h: add include guard. > (findidx): un-nest, make it static inline, add parameters. > * locale/weightwc.h: add include guard, rename findidx to findidxwc. > (findidxwc): un-nest, make it static inline, add parameters. > * posix/fnmatch_loop.c: include weightwc.h or weight.h depending on > WIDE_CHAR_VERSION. Define FINDIDX as findidxwc or findidx. > (FCT): change type of 'extra' to wint_t; do not include weight.h, > un-nest calls to findidx. > * posix/regcomp.c: include weight.h. > (build_equiv_class): don't include weight.h, un-nest findidx. > * posix/regex_internal.h: include weight.h > (re_string_elem_size_at): don't include weight.h, un-nest findidx. > * posix/regexec.c: include weight.h. > (check_node_accept_bytes): don't include weight.h, un-nest findidx. > * string/strcoll_l.c: define FINDIDX, include WEIGHT_H. > (get_next_seq): don't include WEIGHT_H, un-nest findidx. > (get_next_seq_nocache): don't include WEIGHT_H, un-nest findidx. > * string/strxfrm_l.c: define FINDIDX, include WEIGHT_H. > (STRXFRM): don't include WEIGHT_H, un-nest findidx. > * wcsmbs/wcscoll_l.c: define FINDIDX. > * wcsmbs/wcsxfrm_l.c: define FINDIDX.
Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes:
> (FCT): change type of 'extra' to wint_t;
Why do you need this change?
Andreas.
On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: > Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: > >> (FCT): change type of 'extra' to wint_t; > > Why do you need this change? To avoid warnings about different types of argument and parameter > > Andreas. > > -- > Andreas Schwab, SUSE Labs, schwab@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: > On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: >> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >> >>> (FCT): change type of 'extra' to wint_t; >> >> Why do you need this change? > To avoid warnings about different types of argument and parameter But why do you have that conflict in the first place? You must use the type of the object in the end. Andreas.
On Thu, Jul 3, 2014 at 4:26 PM, Andreas Schwab <schwab@suse.de> wrote: > Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: > >> On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: >>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>> >>>> (FCT): change type of 'extra' to wint_t; >>> >>> Why do you need this change? >> To avoid warnings about different types of argument and parameter > > But why do you have that conflict in the first place? You must use the > type of the object in the end. There are multiple uses of findidxwc. Some were using int32_t* for 'extra', some where using wint_t* (e.g. see USTRING_TYPE in wcsmbs/wcscoll.c). When un-nesting findidxwc we have to chose just one type. I chose wint_t because otherwise the change would have been larger. --kcc > > Andreas. > > -- > Andreas Schwab, SUSE Labs, schwab@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
Any other comments? On Thu, Jul 3, 2014 at 5:01 PM, Konstantin Serebryany <konstantin.s.serebryany@gmail.com> wrote: > On Thu, Jul 3, 2014 at 4:26 PM, Andreas Schwab <schwab@suse.de> wrote: >> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >> >>> On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: >>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>> >>>>> (FCT): change type of 'extra' to wint_t; >>>> >>>> Why do you need this change? >>> To avoid warnings about different types of argument and parameter >> >> But why do you have that conflict in the first place? You must use the >> type of the object in the end. > > There are multiple uses of findidxwc. Some were using int32_t* for > 'extra', some where using wint_t* > (e.g. see USTRING_TYPE in wcsmbs/wcscoll.c). > When un-nesting findidxwc we have to chose just one type. I chose > wint_t because otherwise the change would have been larger. > > --kcc > >> >> Andreas. >> >> -- >> Andreas Schwab, SUSE Labs, schwab@suse.de >> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 >> "And now for something completely different."
On 25 July 2014 11:05, Konstantin Serebryany <konstantin.s.serebryany@gmail.com> wrote: > Any other comments? The comments on the include guards are a little odd, usually the name of the define is used rather than the filename. Besides that minor nit the code looks ok to me and also passes make check. I don't think it is something we would want to take at this stage in the freeze however. > On Thu, Jul 3, 2014 at 5:01 PM, Konstantin Serebryany > <konstantin.s.serebryany@gmail.com> wrote: >> On Thu, Jul 3, 2014 at 4:26 PM, Andreas Schwab <schwab@suse.de> wrote: >>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>> >>>> On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: >>>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>>> >>>>>> (FCT): change type of 'extra' to wint_t; >>>>> >>>>> Why do you need this change? >>>> To avoid warnings about different types of argument and parameter >>> >>> But why do you have that conflict in the first place? You must use the >>> type of the object in the end. >> >> There are multiple uses of findidxwc. Some were using int32_t* for >> 'extra', some where using wint_t* >> (e.g. see USTRING_TYPE in wcsmbs/wcscoll.c). >> When un-nesting findidxwc we have to chose just one type. I chose >> wint_t because otherwise the change would have been larger. >> >> --kcc >> >>> >>> Andreas. >>> >>> -- >>> Andreas Schwab, SUSE Labs, schwab@suse.de >>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 >>> "And now for something completely different."
On Fri, Jul 25, 2014 at 3:48 PM, Will Newton <will.newton@linaro.org> wrote: > On 25 July 2014 11:05, Konstantin Serebryany > <konstantin.s.serebryany@gmail.com> wrote: >> Any other comments? > > The comments on the include guards are a little odd, You mean the ChangeLog entry? Should it be like this? 2014-07-25 Kostya Serebryany <konstantin.s.serebryany@gmail.com> * locale/weight.h (_WEIGHT_H_): add include guard. (findidx): un-nest, make it static inline, add parameters. * locale/weightwc.h (_WEIGHTWC_H_): add include guard. (findidx): rename to findidxwc, un-nest, make it static inline, add parameters. * posix/fnmatch_loop.c: include weightwc.h or weight.h depending on WIDE_CHAR_VERSION. Define FINDIDX as findidxwc or findidx. (FCT): change type of 'extra' to wint_t; do not include weight.h, un-nest calls to findidx. * posix/regcomp.c: include weight.h. (build_equiv_class): don't include weight.h, un-nest findidx. * posix/regex_internal.h: include weight.h (re_string_elem_size_at): don't include weight.h, un-nest findidx. * posix/regexec.c: include weight.h. (check_node_accept_bytes): don't include weight.h, un-nest findidx. * string/strcoll_l.c: define FINDIDX, include WEIGHT_H. (get_next_seq): don't include WEIGHT_H, un-nest findidx. (get_next_seq_nocache): don't include WEIGHT_H, un-nest findidx. * string/strxfrm_l.c: define FINDIDX, include WEIGHT_H. (STRXFRM): don't include WEIGHT_H, un-nest findidx. * wcsmbs/wcscoll_l.c: define FINDIDX. * wcsmbs/wcsxfrm_l.c: define FINDIDX. > usually the name > of the define is used rather than the filename. Besides that minor nit > the code looks ok to me and also passes make check. I don't think it > is something we would want to take at this stage in the freeze > however. Ok, let me ping this thread after the freeze is over. Thanks! --kcc > >> On Thu, Jul 3, 2014 at 5:01 PM, Konstantin Serebryany >> <konstantin.s.serebryany@gmail.com> wrote: >>> On Thu, Jul 3, 2014 at 4:26 PM, Andreas Schwab <schwab@suse.de> wrote: >>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>> >>>>> On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: >>>>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>>>> >>>>>>> (FCT): change type of 'extra' to wint_t; >>>>>> >>>>>> Why do you need this change? >>>>> To avoid warnings about different types of argument and parameter >>>> >>>> But why do you have that conflict in the first place? You must use the >>>> type of the object in the end. >>> >>> There are multiple uses of findidxwc. Some were using int32_t* for >>> 'extra', some where using wint_t* >>> (e.g. see USTRING_TYPE in wcsmbs/wcscoll.c). >>> When un-nesting findidxwc we have to chose just one type. I chose >>> wint_t because otherwise the change would have been larger. >>> >>> --kcc >>> >>>> >>>> Andreas. >>>> >>>> -- >>>> Andreas Schwab, SUSE Labs, schwab@suse.de >>>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 >>>> "And now for something completely different." > > > > -- > Will Newton > Toolchain Working Group, Linaro
On 25 July 2014 14:12, Konstantin Serebryany <konstantin.s.serebryany@gmail.com> wrote: > On Fri, Jul 25, 2014 at 3:48 PM, Will Newton <will.newton@linaro.org> wrote: >> On 25 July 2014 11:05, Konstantin Serebryany >> <konstantin.s.serebryany@gmail.com> wrote: >>> Any other comments? >> >> The comments on the include guards are a little odd, > You mean the ChangeLog entry? The ChangeLog looks ok (although sentences should start with capitals) but I was referring to the comments on the include guards: @ -130,3 +136,5 @@ findidx (const unsigned char **cpp, size_t len) /* NOTREACHED */ return 0x43219876; } + +#endif /* weight.h */ Which I would usually expect to see as something like: #endif /* !_WEIGHT_H_ */ But it's a small nit indeed... > Should it be like this? > 2014-07-25 Kostya Serebryany <konstantin.s.serebryany@gmail.com> > > * locale/weight.h (_WEIGHT_H_): add include guard. > (findidx): un-nest, make it static inline, add parameters. > * locale/weightwc.h (_WEIGHTWC_H_): add include guard. > (findidx): rename to findidxwc, un-nest, make it static inline, > add parameters. > * posix/fnmatch_loop.c: include weightwc.h or weight.h depending on > WIDE_CHAR_VERSION. Define FINDIDX as findidxwc or findidx. > (FCT): change type of 'extra' to wint_t; do not include weight.h, > un-nest calls to findidx. > * posix/regcomp.c: include weight.h. > (build_equiv_class): don't include weight.h, un-nest findidx. > * posix/regex_internal.h: include weight.h > (re_string_elem_size_at): don't include weight.h, un-nest findidx. > * posix/regexec.c: include weight.h. > (check_node_accept_bytes): don't include weight.h, un-nest findidx. > * string/strcoll_l.c: define FINDIDX, include WEIGHT_H. > (get_next_seq): don't include WEIGHT_H, un-nest findidx. > (get_next_seq_nocache): don't include WEIGHT_H, un-nest findidx. > * string/strxfrm_l.c: define FINDIDX, include WEIGHT_H. > (STRXFRM): don't include WEIGHT_H, un-nest findidx. > * wcsmbs/wcscoll_l.c: define FINDIDX. > * wcsmbs/wcsxfrm_l.c: define FINDIDX. > > > >> usually the name >> of the define is used rather than the filename. Besides that minor nit >> the code looks ok to me and also passes make check. I don't think it >> is something we would want to take at this stage in the freeze >> however. > > Ok, let me ping this thread after the freeze is over. Thanks! > > --kcc > > >> >>> On Thu, Jul 3, 2014 at 5:01 PM, Konstantin Serebryany >>> <konstantin.s.serebryany@gmail.com> wrote: >>>> On Thu, Jul 3, 2014 at 4:26 PM, Andreas Schwab <schwab@suse.de> wrote: >>>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>>> >>>>>> On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: >>>>>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>>>>> >>>>>>>> (FCT): change type of 'extra' to wint_t; >>>>>>> >>>>>>> Why do you need this change? >>>>>> To avoid warnings about different types of argument and parameter >>>>> >>>>> But why do you have that conflict in the first place? You must use the >>>>> type of the object in the end. >>>> >>>> There are multiple uses of findidxwc. Some were using int32_t* for >>>> 'extra', some where using wint_t* >>>> (e.g. see USTRING_TYPE in wcsmbs/wcscoll.c). >>>> When un-nesting findidxwc we have to chose just one type. I chose >>>> wint_t because otherwise the change would have been larger. >>>> >>>> --kcc >>>> >>>>> >>>>> Andreas. >>>>> >>>>> -- >>>>> Andreas Schwab, SUSE Labs, schwab@suse.de >>>>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 >>>>> "And now for something completely different." >> >> >> >> -- >> Will Newton >> Toolchain Working Group, Linaro
On 07/25/2014 10:29 AM, Will Newton wrote: > +#endif /* weight.h */ > > Which I would usually expect to see as something like: > > #endif /* !_WEIGHT_H_ */ > > But it's a small nit indeed... I would immediately clean this up to !_WEIGHT_H_ if I saw it in code, which means it's a part of our coding style and should be adjusted to match existing style. I like it because it immediately alerts you to the condition that started the conditional. Cheers, Carlos.
I certainly don't care which way to choose, but a quick grep shows that both styles are frequent: % grep '#endif.*/\*.*[A-Z]' */*.h argp/argp-fmtstream.h:#endif /* __OPTIMIZE__ */ argp/argp-fmtstream.h:#endif /* ARGP_FMTSTREAM_USE_LINEWRAP */ argp/argp.h:#endif /* Use extern inlines. */ argp/argp-namefrob.h:#endif /* !_LIBC */ assert/assert.h:#endif /* NDEBUG. */ benchtests/bench-string.h:#endif /* TEST_MAIN */ bits/byteswap.h:#endif /* _BITS_BYTESWAP_H */ bits/environments.h:#endif /* __WORDSIZE == 32 */ bits/mathdef.h:#endif /* ISO C99 */ bits/siginfo.h:#endif /* !have siginfo_t && (have _SIGNAL_H || need siginfo_t). */ bits/siginfo.h:#endif /* have _SIGNAL_H. */ bits/sigset.h:#endif /* ! _SIGSET_H_fns. */ ... 248 total % grep '#endif.*/\*.*\.h' */*.h argp/argp-fmtstream.h:#endif /* argp-fmtstream.h */ argp/argp.h:#endif /* argp.h */ assert/assert.h:#endif /* assert.h */ b/abi-versions.h:#endif /* abi-versions.h */ bits/atomic.h:#endif /* bits/atomic.h */ bits/ipctypes.h:#endif /* bits/ipctypes.h */ bits/libc-lock.h:#endif /* bits/libc-lock.h */ bits/libc-tsd.h:#endif /* bits/libc-tsd.h */ bits/signum.h:#endif /* <signal.h> included. */ bits/sigthread.h:#endif /* bits/sigthread.h */ bits/sockaddr.h:#endif /* bits/sockaddr.h */ bits/socket.h:#endif /* bits/socket.h */ 184 total --kcc On Fri, Jul 25, 2014 at 10:21 PM, Carlos O'Donell <carlos@redhat.com> wrote: > On 07/25/2014 10:29 AM, Will Newton wrote: >> +#endif /* weight.h */ >> >> Which I would usually expect to see as something like: >> >> #endif /* !_WEIGHT_H_ */ >> >> But it's a small nit indeed... > > I would immediately clean this up to !_WEIGHT_H_ if I saw > it in code, which means it's a part of our coding style and > should be adjusted to match existing style. > > I like it because it immediately alerts you to the condition > that started the conditional. > > Cheers, > Carlos.
On 07/25/2014 03:19 PM, Konstantin Serebryany wrote: > I certainly don't care which way to choose, but a quick grep shows > that both styles are frequent: This isn't quite an apples-to-apples comparison. For commenting conditionals I think it's overwhelmingly the case that the closing #endif contains the conditional that was true for that branch. The case of conditionals that are file inclusion guards is unique though, and it turns out I'm wrong here. In this case the overwhelming majority close out the conditional with the name of the file. This makes sense since it gives the most information to the developer that it is (a) an inclusion guard and (b) which file. The down side being that if the file is moved you need to update the inclusion guard comment. I think both are acceptable and I've commented as such in the Style and Conventions guide. https://sourceware.org/glibc/wiki/Style_and_Conventions#Commenting_.23endif I retract my statement that I'd change your patch. Cheers, Carlos.
2.20 has been released. Can we now proceed with this patch? (I've double-checked, it still applies, builds and validates) Thanks, --kcc On Fri, Jul 25, 2014 at 6:12 AM, Konstantin Serebryany <konstantin.s.serebryany@gmail.com> wrote: > On Fri, Jul 25, 2014 at 3:48 PM, Will Newton <will.newton@linaro.org> wrote: >> On 25 July 2014 11:05, Konstantin Serebryany >> <konstantin.s.serebryany@gmail.com> wrote: >>> Any other comments? >> >> The comments on the include guards are a little odd, > You mean the ChangeLog entry? > > Should it be like this? > 2014-07-25 Kostya Serebryany <konstantin.s.serebryany@gmail.com> > > * locale/weight.h (_WEIGHT_H_): add include guard. > (findidx): un-nest, make it static inline, add parameters. > * locale/weightwc.h (_WEIGHTWC_H_): add include guard. > (findidx): rename to findidxwc, un-nest, make it static inline, > add parameters. > * posix/fnmatch_loop.c: include weightwc.h or weight.h depending on > WIDE_CHAR_VERSION. Define FINDIDX as findidxwc or findidx. > (FCT): change type of 'extra' to wint_t; do not include weight.h, > un-nest calls to findidx. > * posix/regcomp.c: include weight.h. > (build_equiv_class): don't include weight.h, un-nest findidx. > * posix/regex_internal.h: include weight.h > (re_string_elem_size_at): don't include weight.h, un-nest findidx. > * posix/regexec.c: include weight.h. > (check_node_accept_bytes): don't include weight.h, un-nest findidx. > * string/strcoll_l.c: define FINDIDX, include WEIGHT_H. > (get_next_seq): don't include WEIGHT_H, un-nest findidx. > (get_next_seq_nocache): don't include WEIGHT_H, un-nest findidx. > * string/strxfrm_l.c: define FINDIDX, include WEIGHT_H. > (STRXFRM): don't include WEIGHT_H, un-nest findidx. > * wcsmbs/wcscoll_l.c: define FINDIDX. > * wcsmbs/wcsxfrm_l.c: define FINDIDX. > > > >> usually the name >> of the define is used rather than the filename. Besides that minor nit >> the code looks ok to me and also passes make check. I don't think it >> is something we would want to take at this stage in the freeze >> however. > > Ok, let me ping this thread after the freeze is over. Thanks! > > --kcc > > >> >>> On Thu, Jul 3, 2014 at 5:01 PM, Konstantin Serebryany >>> <konstantin.s.serebryany@gmail.com> wrote: >>>> On Thu, Jul 3, 2014 at 4:26 PM, Andreas Schwab <schwab@suse.de> wrote: >>>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>>> >>>>>> On Thu, Jul 3, 2014 at 4:06 PM, Andreas Schwab <schwab@suse.de> wrote: >>>>>>> Konstantin Serebryany <konstantin.s.serebryany@gmail.com> writes: >>>>>>> >>>>>>>> (FCT): change type of 'extra' to wint_t; >>>>>>> >>>>>>> Why do you need this change? >>>>>> To avoid warnings about different types of argument and parameter >>>>> >>>>> But why do you have that conflict in the first place? You must use the >>>>> type of the object in the end. >>>> >>>> There are multiple uses of findidxwc. Some were using int32_t* for >>>> 'extra', some where using wint_t* >>>> (e.g. see USTRING_TYPE in wcsmbs/wcscoll.c). >>>> When un-nesting findidxwc we have to chose just one type. I chose >>>> wint_t because otherwise the change would have been larger. >>>> >>>> --kcc >>>> >>>>> >>>>> Andreas. >>>>> >>>>> -- >>>>> Andreas Schwab, SUSE Labs, schwab@suse.de >>>>> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 >>>>> "And now for something completely different." >> >> >> >> -- >> Will Newton >> Toolchain Working Group, Linaro
When it's been very long at all, you should post a fresh patch that's verified against today's trunk, and not include any quoted nonsense in the message. Just post in the same thread and that should be enough context for everybody.
diff --git a/locale/weight.h b/locale/weight.h index 9eb8ac6..485526c 100644 --- a/locale/weight.h +++ b/locale/weight.h @@ -16,10 +16,16 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#ifndef _WEIGHT_H_ +#define _WEIGHT_H_ + /* Find index of weight. */ -auto inline int32_t +static inline int32_t __attribute ((always_inline)) -findidx (const unsigned char **cpp, size_t len) +findidx (const int32_t *table, + const int32_t *indirect, + const unsigned char *extra, + const unsigned char **cpp, size_t len) { int_fast32_t i = table[*(*cpp)++]; const unsigned char *cp; @@ -130,3 +136,5 @@ findidx (const unsigned char **cpp, size_t len) /* NOTREACHED */ return 0x43219876; } + +#endif /* weight.h */ diff --git a/locale/weightwc.h b/locale/weightwc.h index 8f047e3..3348544 100644 --- a/locale/weightwc.h +++ b/locale/weightwc.h @@ -16,10 +16,16 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#ifndef _WEIGHTWC_H_ +#define _WEIGHTWC_H_ + /* Find index of weight. */ -auto inline int32_t +static inline int32_t __attribute ((always_inline)) -findidx (const wint_t **cpp, size_t len) +findidxwc (const int32_t *table, + const int32_t *indirect, + const wint_t *extra, + const wint_t **cpp, size_t len) { wint_t ch = *(*cpp)++; int32_t i = __collidx_table_lookup ((const char *) table, ch); @@ -109,3 +115,5 @@ findidx (const wint_t **cpp, size_t len) /* NOTREACHED */ return 0x43219876; } + +#endif /* weightwc.h */ diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c index f79d051..8cd2f31 100644 --- a/posix/fnmatch_loop.c +++ b/posix/fnmatch_loop.c @@ -17,6 +17,17 @@ #include <stdint.h> +# if WIDE_CHAR_VERSION +# include <locale/weightwc.h> +# undef FINDIDX +# define FINDIDX findidxwc +# else +# include <locale/weight.h> +# undef FINDIDX +# define FINDIDX findidx +# endif + + struct STRUCT { const CHAR *pattern; @@ -376,7 +387,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used) const int32_t *table; # if WIDE_CHAR_VERSION const int32_t *weights; - const int32_t *extra; + const wint_t *extra; # else const unsigned char *weights; const unsigned char *extra; @@ -385,19 +396,12 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used) int32_t idx; const UCHAR *cp = (const UCHAR *) str; - /* This #include defines a local function! */ -# if WIDE_CHAR_VERSION -# include <locale/weightwc.h> -# else -# include <locale/weight.h> -# endif - # if WIDE_CHAR_VERSION table = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_TABLEWC); weights = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_WEIGHTWC); - extra = (const int32_t *) + extra = (const wint_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_EXTRAWC); indirect = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTWC); @@ -412,7 +416,7 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTMB); # endif - idx = findidx (&cp, 1); + idx = FINDIDX (table, indirect, extra, &cp, 1); if (idx != 0) { /* We found a table entry. Now see whether the @@ -422,7 +426,8 @@ FCT (pattern, string, string_end, no_leading_period, flags, ends, alloca_used) int32_t idx2; const UCHAR *np = (const UCHAR *) n; - idx2 = findidx (&np, string_end - n); + idx2 = FINDIDX (table, indirect, extra, + &np, string_end - n); if (idx2 != 0 && (idx >> 24) == (idx2 >> 24) && len == weights[idx2 & 0xffffff]) diff --git a/posix/regcomp.c b/posix/regcomp.c index 921d0f4..b1cc8bc 100644 --- a/posix/regcomp.c +++ b/posix/regcomp.c @@ -3389,6 +3389,8 @@ parse_bracket_symbol (bracket_elem_t *elem, re_string_t *regexp, return REG_NOERROR; } +#include <locale/weight.h> + /* Helper function for parse_bracket_exp. Build the equivalence class which is represented by NAME. The result are written to MBCSET and SBCSET. @@ -3413,8 +3415,6 @@ build_equiv_class (bitset_t sbcset, const unsigned char *name) int32_t idx1, idx2; unsigned int ch; size_t len; - /* This #include defines a local function! */ -# include <locale/weight.h> /* Calculate the index for equivalence class. */ cp = name; table = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_TABLEMB); @@ -3424,7 +3424,7 @@ build_equiv_class (bitset_t sbcset, const unsigned char *name) _NL_COLLATE_EXTRAMB); indirect = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTMB); - idx1 = findidx (&cp, -1); + idx1 = findidx (table, indirect, extra, &cp, -1); if (BE (idx1 == 0 || *cp != '\0', 0)) /* This isn't a valid character. */ return REG_ECOLLATE; @@ -3435,7 +3435,7 @@ build_equiv_class (bitset_t sbcset, const unsigned char *name) { char_buf[0] = ch; cp = char_buf; - idx2 = findidx (&cp, 1); + idx2 = findidx (table, indirect, extra, &cp, 1); /* idx2 = table[ch]; */ diff --git a/posix/regex_internal.h b/posix/regex_internal.h index 75c390f..c38980b 100644 --- a/posix/regex_internal.h +++ b/posix/regex_internal.h @@ -733,6 +733,8 @@ re_string_wchar_at (const re_string_t *pstr, int idx) } # ifndef NOT_IN_libc +# include <locale/weight.h> + static int internal_function __attribute__ ((pure, unused)) re_string_elem_size_at (const re_string_t *pstr, int idx) @@ -740,7 +742,6 @@ re_string_elem_size_at (const re_string_t *pstr, int idx) # ifdef _LIBC const unsigned char *p, *extra; const int32_t *table, *indirect; -# include <locale/weight.h> uint_fast32_t nrules = _NL_CURRENT_WORD (LC_COLLATE, _NL_COLLATE_NRULES); if (nrules != 0) @@ -751,7 +752,7 @@ re_string_elem_size_at (const re_string_t *pstr, int idx) indirect = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTMB); p = pstr->mbs + idx; - findidx (&p, pstr->len - idx); + findidx (table, indirect, extra, &p, pstr->len - idx); return p - pstr->mbs - idx; } else diff --git a/posix/regexec.c b/posix/regexec.c index 7032da7..3e52682 100644 --- a/posix/regexec.c +++ b/posix/regexec.c @@ -3749,6 +3749,8 @@ group_nodes_into_DFAstates (const re_dfa_t *dfa, const re_dfastate_t *state, one collating element like '.', '[a-z]', opposite to the other nodes can only accept one byte. */ +#include <locale/weight.h> + static int internal_function check_node_accept_bytes (const re_dfa_t *dfa, int node_idx, @@ -3868,8 +3870,6 @@ check_node_accept_bytes (const re_dfa_t *dfa, int node_idx, const int32_t *table, *indirect; const unsigned char *weights, *extra; const char *collseqwc; - /* This #include defines a local function! */ -# include <locale/weight.h> /* match with collating_symbol? */ if (cset->ncoll_syms) @@ -3925,7 +3925,7 @@ check_node_accept_bytes (const re_dfa_t *dfa, int node_idx, _NL_CURRENT (LC_COLLATE, _NL_COLLATE_EXTRAMB); indirect = (const int32_t *) _NL_CURRENT (LC_COLLATE, _NL_COLLATE_INDIRECTMB); - int32_t idx = findidx (&cp, elem_len); + int32_t idx = findidx (table, indirect, extra, &cp, elem_len); if (idx > 0) for (i = 0; i < cset->nequiv_classes; ++i) { diff --git a/string/strcoll_l.c b/string/strcoll_l.c index 10ce4a6..8764e67 100644 --- a/string/strcoll_l.c +++ b/string/strcoll_l.c @@ -33,6 +33,7 @@ # define STRCMP strcmp # define STRLEN strlen # define WEIGHT_H "../locale/weight.h" +# define FINDIDX findidx # define SUFFIX MB # define L(arg) arg #endif @@ -146,13 +147,14 @@ get_next_seq_cached (coll_seq *seq, int nrules, int pass, seq->idxnow = idxnow; } +#include WEIGHT_H + /* Get next sequence. Traverse the string as required. */ static void get_next_seq (coll_seq *seq, int nrules, const unsigned char *rulesets, const USTRING_TYPE *weights, const int32_t *table, const USTRING_TYPE *extra, const int32_t *indirect) { -#include WEIGHT_H size_t val = seq->val = 0; int len = seq->len; size_t backw_stop = seq->backw_stop; @@ -194,7 +196,7 @@ get_next_seq (coll_seq *seq, int nrules, const unsigned char *rulesets, while (*us != L('\0')) { - int32_t tmp = findidx (&us, -1); + int32_t tmp = FINDIDX (table, indirect, extra, &us, -1); rulearr[idxmax] = tmp >> 24; idxarr[idxmax] = tmp & 0xffffff; idxcnt = idxmax++; @@ -242,7 +244,6 @@ get_next_seq_nocache (coll_seq *seq, int nrules, const unsigned char *rulesets, const USTRING_TYPE *extra, const int32_t *indirect, int pass) { -#include WEIGHT_H size_t val = seq->val = 0; int len = seq->len; size_t backw_stop = seq->backw_stop; @@ -285,7 +286,7 @@ get_next_seq_nocache (coll_seq *seq, int nrules, const unsigned char *rulesets, us = seq->back_us; while (i < backw) { - int32_t tmp = findidx (&us, -1); + int32_t tmp = FINDIDX (table, indirect, extra, &us, -1); idx = tmp & 0xffffff; i++; } @@ -300,7 +301,7 @@ get_next_seq_nocache (coll_seq *seq, int nrules, const unsigned char *rulesets, while (*us != L('\0')) { - int32_t tmp = findidx (&us, -1); + int32_t tmp = FINDIDX (table, indirect, extra, &us, -1); unsigned char rule = tmp >> 24; prev_idx = idx; idx = tmp & 0xffffff; diff --git a/string/strxfrm_l.c b/string/strxfrm_l.c index 04b9338..7d41a28 100644 --- a/string/strxfrm_l.c +++ b/string/strxfrm_l.c @@ -33,6 +33,7 @@ # define STRLEN strlen # define STPNCPY __stpncpy # define WEIGHT_H "../locale/weight.h" +# define FINDIDX findidx # define SUFFIX MB # define L(arg) arg #endif @@ -80,6 +81,7 @@ utf8_encode (char *buf, int val) } #endif +#include WEIGHT_H size_t STRXFRM (STRING_TYPE *dest, const STRING_TYPE *src, size_t n, __locale_t l) @@ -104,8 +106,6 @@ STRXFRM (STRING_TYPE *dest, const STRING_TYPE *src, size_t n, __locale_t l) size_t idxcnt; int use_malloc; -#include WEIGHT_H - if (nrules == 0) { if (n != 0) @@ -174,7 +174,7 @@ STRXFRM (STRING_TYPE *dest, const STRING_TYPE *src, size_t n, __locale_t l) idxmax = 0; do { - int32_t tmp = findidx (&usrc, -1); + int32_t tmp = FINDIDX (table, indirect, extra, &usrc, -1); rulearr[idxmax] = tmp >> 24; idxarr[idxmax] = tmp & 0xffffff; diff --git a/wcsmbs/wcscoll_l.c b/wcsmbs/wcscoll_l.c index 74e2e39..a402806 100644 --- a/wcsmbs/wcscoll_l.c +++ b/wcsmbs/wcscoll_l.c @@ -26,6 +26,7 @@ #define STRCMP wcscmp #define STRLEN __wcslen #define WEIGHT_H "../locale/weightwc.h" +#define FINDIDX findidxwc #define SUFFIX WC #define L(arg) L##arg #define WIDE_CHAR_VERSION 1 diff --git a/wcsmbs/wcsxfrm_l.c b/wcsmbs/wcsxfrm_l.c index f3f3f50..23e1703 100644 --- a/wcsmbs/wcsxfrm_l.c +++ b/wcsmbs/wcsxfrm_l.c @@ -26,6 +26,7 @@ #define STRLEN __wcslen #define STPNCPY __wcpncpy #define WEIGHT_H "../locale/weightwc.h" +#define FINDIDX findidxwc #define SUFFIX WC #define L(arg) L##arg #define WIDE_CHAR_VERSION 1