* Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)? @ 2021-08-19 16:26 Roger Sayle 2021-08-20 8:32 ` Corinna Vinschen 0 siblings, 1 reply; 4+ messages in thread From: Roger Sayle @ 2021-08-19 16:26 UTC (permalink / raw) To: 'Newlib' I believe that newlib/libc/locale/Makefile.am should place lnumeric.c in GENERAL_SOURCES [EL/IX level 1]. The motivation for doing this is that both stdlib/strtod.c and stdlib/gdtoa-gethex.c unconditionally call __get_numeric_locale, and they themselves are in GENERAL_SOURCES (i.e. EL/IX level 1). This would fix another unresolved symbol problem on nvptx-none. Many thanks in advance, Best regards, Roger -- Roger Sayle NextMove Software Cambridge, UK ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)? 2021-08-19 16:26 Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)? Roger Sayle @ 2021-08-20 8:32 ` Corinna Vinschen 2021-08-20 13:29 ` Joel Sherrill 0 siblings, 1 reply; 4+ messages in thread From: Corinna Vinschen @ 2021-08-20 8:32 UTC (permalink / raw) To: Roger Sayle; +Cc: 'Newlib' On Aug 19 17:26, Roger Sayle wrote: > > > I believe that newlib/libc/locale/Makefile.am should place lnumeric.c > > in GENERAL_SOURCES [EL/IX level 1]. The motivation for doing this is > > that both stdlib/strtod.c and stdlib/gdtoa-gethex.c unconditionally > > call __get_numeric_locale, and they themselves are in GENERAL_SOURCES > > (i.e. EL/IX level 1). Hmm, I wonder if we shouldn't rather fix this in strtod.c and gdtoa-gethex.c. If the target doesn't define __HAVE_LOCALE_INFO__, there's not much of a point to call __get_numeric_locale(loc). Rather, decimal_point could just be set to the dot, i.e. diff --git a/newlib/libc/stdlib/gdtoa-gethex.c b/newlib/libc/stdlib/gdtoa-gethex.c index 1d3da2889ea1..74f30e69013d 100644 --- a/newlib/libc/stdlib/gdtoa-gethex.c +++ b/newlib/libc/stdlib/gdtoa-gethex.c @@ -149,10 +149,16 @@ gethex (struct _reent *ptr, const char **sp, const FPI *fpi, int esign, havedig, irv, k, n, nbits, up, zret; __ULong L, lostbits, *x; Long e, e1; - const unsigned char *decimalpoint = (unsigned char *) +#ifdef __HAVE_LOCALE_INFO__ + const unsigned char *decimalpoint = (const unsigned char *) __get_numeric_locale(loc)->decimal_point; - size_t decp_len = strlen ((const char *) decimalpoint); - unsigned char decp_end = decimalpoint[decp_len - 1]; + const size_t decp_len = strlen ((const char *) decimalpoint); + const unsigned char decp_end = decimalpoint[decp_len - 1]; +#else + const unsigned char *decimalpoint = (const unsigned char *) "."; + const size_t decp_len = 1; + const unsigned char decp_end = (unsigned char) '.'; +#endif havedig = 0; s0 = *(const unsigned char **)sp + 2; diff --git a/newlib/libc/stdlib/strtod.c b/newlib/libc/stdlib/strtod.c index 019416ca7026..ca6d79040224 100644 --- a/newlib/libc/stdlib/strtod.c +++ b/newlib/libc/stdlib/strtod.c @@ -263,7 +263,11 @@ _strtod_l (struct _reent *ptr, const char *__restrict s00, char **__restrict se, #ifdef Honor_FLT_ROUNDS int rounding; #endif +#ifdef __HAVE_LOCALE_INFO__ const char *decimal_point = __get_numeric_locale(loc)->decimal_point; +#else + const char *decimal_point = "."; +#endif int dec_len = strlen (decimal_point); delta = bs = bd = NULL; This would also avoid to pull in a readonly struct which is never used. Thoughts? Corinna ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)? 2021-08-20 8:32 ` Corinna Vinschen @ 2021-08-20 13:29 ` Joel Sherrill 2021-08-23 8:03 ` Corinna Vinschen 0 siblings, 1 reply; 4+ messages in thread From: Joel Sherrill @ 2021-08-20 13:29 UTC (permalink / raw) To: Newlib, Roger Sayle On Fri, Aug 20, 2021 at 3:32 AM Corinna Vinschen <vinschen@redhat.com> wrote: > > On Aug 19 17:26, Roger Sayle wrote: > > > > > > I believe that newlib/libc/locale/Makefile.am should place lnumeric.c > > > > in GENERAL_SOURCES [EL/IX level 1]. The motivation for doing this is > > > > that both stdlib/strtod.c and stdlib/gdtoa-gethex.c unconditionally > > > > call __get_numeric_locale, and they themselves are in GENERAL_SOURCES > > > > (i.e. EL/IX level 1). The FACE Technical Standard (https://opengroup.org/face) has four POSIX profiles which were designed with security and avionics certification in mind has no locale support in the lowest three profiles and only limited support in the largest profile. The Software Communications Architecture (SCA) has a few profiles and targets software defined radios (https://sds.wirelessinnovation.org/history-of-the-sca). It does not include any locale support. The intended users would likely also be subject to security audits. With that background, making locale as optional as possible is a good idea for footprint both in code size and what might need to be audited. I'm happy with the patch. --joel > > Hmm, I wonder if we shouldn't rather fix this in strtod.c and > gdtoa-gethex.c. If the target doesn't define __HAVE_LOCALE_INFO__, > there's not much of a point to call __get_numeric_locale(loc). > Rather, decimal_point could just be set to the dot, i.e. > > diff --git a/newlib/libc/stdlib/gdtoa-gethex.c b/newlib/libc/stdlib/gdtoa-gethex.c > index 1d3da2889ea1..74f30e69013d 100644 > --- a/newlib/libc/stdlib/gdtoa-gethex.c > +++ b/newlib/libc/stdlib/gdtoa-gethex.c > @@ -149,10 +149,16 @@ gethex (struct _reent *ptr, const char **sp, const FPI *fpi, > int esign, havedig, irv, k, n, nbits, up, zret; > __ULong L, lostbits, *x; > Long e, e1; > - const unsigned char *decimalpoint = (unsigned char *) > +#ifdef __HAVE_LOCALE_INFO__ > + const unsigned char *decimalpoint = (const unsigned char *) > __get_numeric_locale(loc)->decimal_point; > - size_t decp_len = strlen ((const char *) decimalpoint); > - unsigned char decp_end = decimalpoint[decp_len - 1]; > + const size_t decp_len = strlen ((const char *) decimalpoint); > + const unsigned char decp_end = decimalpoint[decp_len - 1]; > +#else > + const unsigned char *decimalpoint = (const unsigned char *) "."; > + const size_t decp_len = 1; > + const unsigned char decp_end = (unsigned char) '.'; > +#endif > > havedig = 0; > s0 = *(const unsigned char **)sp + 2; > diff --git a/newlib/libc/stdlib/strtod.c b/newlib/libc/stdlib/strtod.c > index 019416ca7026..ca6d79040224 100644 > --- a/newlib/libc/stdlib/strtod.c > +++ b/newlib/libc/stdlib/strtod.c > @@ -263,7 +263,11 @@ _strtod_l (struct _reent *ptr, const char *__restrict s00, char **__restrict se, > #ifdef Honor_FLT_ROUNDS > int rounding; > #endif > +#ifdef __HAVE_LOCALE_INFO__ > const char *decimal_point = __get_numeric_locale(loc)->decimal_point; > +#else > + const char *decimal_point = "."; > +#endif > int dec_len = strlen (decimal_point); > > delta = bs = bd = NULL; > > This would also avoid to pull in a readonly struct which is never used. > > > Thoughts? > > > Corinna > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)? 2021-08-20 13:29 ` Joel Sherrill @ 2021-08-23 8:03 ` Corinna Vinschen 0 siblings, 0 replies; 4+ messages in thread From: Corinna Vinschen @ 2021-08-23 8:03 UTC (permalink / raw) To: newlib On Aug 20 08:29, Joel Sherrill wrote: > On Fri, Aug 20, 2021 at 3:32 AM Corinna Vinschen <vinschen@redhat.com> wrote: > > > > On Aug 19 17:26, Roger Sayle wrote: > > > > > > > > > I believe that newlib/libc/locale/Makefile.am should place lnumeric.c > > > > > > in GENERAL_SOURCES [EL/IX level 1]. The motivation for doing this is > > > > > > that both stdlib/strtod.c and stdlib/gdtoa-gethex.c unconditionally > > > > > > call __get_numeric_locale, and they themselves are in GENERAL_SOURCES > > > > > > (i.e. EL/IX level 1). > > The FACE Technical Standard (https://opengroup.org/face) has four POSIX profiles > which were designed with security and avionics certification in mind has no > locale support in the lowest three profiles and only limited support > in the largest > profile. > > The Software Communications Architecture (SCA) has a few profiles and targets > software defined radios > (https://sds.wirelessinnovation.org/history-of-the-sca). It > does not include any locale support. The intended users would likely also be > subject to security audits. > > With that background, making locale as optional as possible is a good idea for > footprint both in code size and what might need to be audited. I'm happy with > the patch. Pushed. Thanks, Corinna ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-23 8:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-19 16:26 Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)? Roger Sayle 2021-08-20 8:32 ` Corinna Vinschen 2021-08-20 13:29 ` Joel Sherrill 2021-08-23 8:03 ` Corinna Vinschen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).