Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)?

Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)?

[Roger Sayle]

 

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

 

Re: Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)?

[Corinna Vinschen]

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

Re: Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)?

[Joel Sherrill]

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
>

Re: Should libc/locale/lnumeric.c be in GENERAL_SOURCES (EL/IX level 1)?

[Corinna Vinschen]

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