Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable glibc undocumented querylocale() #22701

Open
wants to merge 1 commit into
base: maint-5.38
Choose a base branch
from
+8 −11
Open

Enable glibc undocumented querylocale() #22701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions intrpvar.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,18 +748,12 @@ PERLVAR(I, padix_floor, PADOFFSET) /* how low may inner block reset padix */
#if defined(USE_POSIX_2008_LOCALE) && defined(MULTIPLICITY)
PERLVARI(I, cur_locale_obj, locale_t, NULL)
#endif
#ifdef USE_PL_CURLOCALES

/* This is the most number of categories we've encountered so far on any
* platform, doesn't include LC_ALL */
PERLVARA(I, curlocales, 12, const char *)

#endif
#ifdef USE_PL_CUR_LC_ALL

PERLVARI(I, cur_LC_ALL, const char *, NULL)

#endif
Comment on lines -751 to -762
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this change the layout for a perl built with -DNO_LOCALE?

Even with that fixed I'm hesitant to backport this change - I'm not sure it qualifies under the conditions from perlpolicy, and it appears to be a minor enhancement at best.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#21366, which this fixes, is a regression introduced in 5.38. Why would that not be something we need to fix? I don't understand about this being an "enhancement". It is a bare minimum fix.

I believe you are right about NO_LOCALE, and it is fixable as you surmised. I would want to make sure there is no other combination that would need to be addressed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I should have followed through the reference you provided.

I think it's reasonable to apply this is the bincompat is fixed.

I do worry about systems that don't have a querylocale() work-a-like (are there any? musl provides this extension) -- if I understand the issue this won't fix such systems.

#ifdef USE_LOCALE_COLLATE

/* The emory needed to store the collxfrm transformation of a string with
Expand Down
6 changes: 5 additions & 1 deletion makedef.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,11 @@ BEGIN
}
}

if ($define{USE_POSIX_2008_LOCALE} && $define{HAS_QUERYLOCALE})
if ( $define{USE_POSIX_2008_LOCALE}
&& ( $define{HAS_QUERYLOCALE}
|| ( $Config{cppsymbols} =~ /__GLIBC__/
&& $define{HAS_NL_LANGINFO_L}
&& ! $define{SETLOCALE_ACCEPTS_ANY_LOCALE_NAME})))
{
$define{USE_QUERYLOCALE} = 1;

Expand Down
7 changes: 3 additions & 4 deletions perl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1287,14 +1287,13 @@ violations are fatal.
# endif

# include "perl_langinfo.h" /* Needed for _NL_LOCALE_NAME */

# ifdef USE_POSIX_2008_LOCALE

# if defined(HAS_QUERYLOCALE) \
/* Use querylocale if has it, or has the glibc internal \
* undocumented equivalent. */ \
* undocumented equivalent (if not forbidden). */ \
|| ( defined(_NL_LOCALE_NAME) \
/* And asked for */ \
&& defined(USE_NL_LOCALE_NAME) \
&& ! defined(NO_USE_NL_LOCALE_NAME) \
/* nl_langinfo_l almost certainly will exist on systems that \
* have _NL_LOCALE_NAME, so there is nothing lost by \
* requiring it instead of also allowing plain nl_langinfo(). \
Expand Down
Loading