[STEP,3] remove access of locale

Message ID 004001d3a1b8$95e2a7b0$c1a7f710$@jasoon.nl
State New
Headers show
Series
  • [STEP,3] remove access of locale
Related show

Commit Message

Jaap de Wolff Feb. 9, 2018, 3:13 p.m.
The functions that can be used to create or modify locales are adapted.
Functions to create or duplicate a locale are returning NULL now.
Should they also set an errno?
And if they should, what errno should they set?

I used a simple project to test the memory usage:

compiled using GNU Tools ARM Embedded version 7 2017-q4-major
   text	   data	    bss	    dec	    hex	filename
   4800	    524	    416	   5740	   166c	Test.elf

Same code compiled and linked using adapted newlib-nano, with all proposed
patches:
   text	   data	    bss	    dec	    hex	filename
   4489	    160	    416	   5065	   13c9	Test.elf

Jaap de Wolff

 /* In POSIX terms the current locale is the locale used by all functions
@@ -227,7 +235,11 @@ __get_locale_r (struct _reent *r)
 _ELIDABLE_INLINE struct __locale_t *
 __get_current_locale (void)
 {
+#ifdef _REENT_SMALL
+  return NULL;
+#else
   return _REENT->_locale ?: __get_global_locale ();
+#endif	
 }
 
 /* Only access fixed "C" locale using this function.  Fake for !_MB_CAPABLE
@@ -276,14 +288,18 @@ __get_current_ctype_locale (void)
 _ELIDABLE_INLINE int
 __locale_mb_cur_max_l (struct __locale_t *locale)
 {
+#ifdef _REENT_SMALL
+	return 2;
+#else	
 #ifdef __HAVE_LOCALE_INFO__
   return __get_ctype_locale (locale)->mb_cur_max[0];
 #else
   return locale->mb_cur_max[0];
 #endif
+#endif
 }
 
-#ifdef __HAVE_LOCALE_INFO__
+#if defined (__HAVE_LOCALE_INFO__) && !defined(_REENT_SMALL)
 _ELIDABLE_INLINE const struct lc_monetary_T *
 __get_monetary_locale (struct __locale_t *locale)
 {
@@ -385,6 +401,7 @@ __get_current_messages_locale (void)
 }
 #endif /* !__HAVE_LOCALE_INFO__ */
 
+#ifndef _REENT_SMALL
 _ELIDABLE_INLINE const char *
 __locale_charset (struct __locale_t *locale)
 {
@@ -420,6 +437,7 @@ __locale_cjk_lang (void)
 {
   return __get_current_locale ()->cjk_lang;
 }
+#endif	/* NO_LOCALE */
 
 int __ctype_load_locale (struct __locale_t *, const char *, void *,
 			 const char *, int);
@@ -439,11 +457,23 @@ extern void __set_charset_from_locale (const char
*locale, char *charset);
 extern char *__set_locale_from_locale_alias (const char *, char *);
 #endif
 
+#ifdef _REENT_SMALL
+const char *loc_get_decimalpoint(void *locale);
+#ifdef __CYGWIN__
+#define loc_wctomb(loc,r,buff,wc,ps) __utf8_wctomb(r,buff,wc,ps)
+#define cur_loc_wctomb __utf8_wctomb
+#define cur_loc_mbtowc __utf8_mbtowc
+#else
+#define loc_wctomb(loc,r,buff,wc,ps) __ascii_wctomb(r,buff,wc,ps)
+#define cur_loc_wctomb __ascii_wctomb
+#define cur_loc_mbtowc __ascii_mbtowc
+#endif /* __CYGWIN__ */
+#else /*  _REENT_SMALL */
 #define loc_get_decimalpoint(loc) (__localeconv_l(loc))->decimal_point
 #define loc_wctomb(loc,r,buff,wc,ps) (loc)->wctomb(r,buff,wc,ps)
 #define cur_loc_wctomb __get_current_locale ()->wctomb
 #define cur_loc_mbtowc __get_current_locale ()->mbtowc
-
+#endif /*  _REENT_SMALL */
 __END_DECLS
 
 #endif /* !_SETLOCALE_H_ */

Comments

Eric Blake Feb. 9, 2018, 6:46 p.m. | #1
On 02/09/2018 09:13 AM, Jaap de Wolff wrote:
> The functions that can be used to create or modify locales are adapted.

> Functions to create or duplicate a locale are returning NULL now.

> Should they also set an errno?


Yes.

> And if they should, what errno should they set?


>   _duplocale_r (struct _reent *p, struct __locale_t *locobj)

>   {

> +#ifdef _REENT_SMALL

> +	return NULL;


This should set ENOMEM (conceptually, we're so small that ALL attempts 
to create a locale are beyond the memory limits we have available ;)

By the way, POSIX says duplocale returns '(locale_)0' on failure; you're 
lucky that NULL works here because our locale_t is a wrapper around 
'struct __locale_t*'.


>   _newlocale_r (struct _reent *p, int category_mask, const char *locale,

>   	      struct __locale_t *base)

>   {

> +#ifdef _REENT_SMALL

> +	return NULL;

> +#else	


ENOMEM is also appropriate here

> +++ b/newlib/libc/locale/setlocale.h

> @@ -209,15 +209,23 @@ extern size_t _wcsnrtombs_l (struct _reent *, char *,

> const wchar_t **,

>   _ELIDABLE_INLINE struct __locale_t *

>   __get_global_locale ()

>   {

> +#ifdef _REENT_SMALL

> +  return NULL;


I'm not sure how this internal function will affect things, or whether 
it should return NULL. Ideally, in a small compilation environment, we 
shouldn't even be dereferencing __get_global_locale because we should 
already know things are hard-coded; if it does dereference NULL, you 
have a problem.  So do you really want _REENT_SMALL code here, or is it 
better to omit the function entirely?

> @@ -276,14 +288,18 @@ __get_current_ctype_locale (void)

>   _ELIDABLE_INLINE int

>   __locale_mb_cur_max_l (struct __locale_t *locale)

>   {

> +#ifdef _REENT_SMALL

> +	return 2;


Why 2?  The "C" locale should return 1, and if you aren't supporting 
locales, you shouldn't have any multi-byte sequences longer than 1.


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
Corinna Vinschen Feb. 9, 2018, 7:41 p.m. | #2
On Feb  9 12:46, Eric Blake wrote:
> On 02/09/2018 09:13 AM, Jaap de Wolff wrote:

> > The functions that can be used to create or modify locales are adapted.

> > Functions to create or duplicate a locale are returning NULL now.

> > Should they also set an errno?

> 

> Yes.

> 

> > And if they should, what errno should they set?

> 

> >   _duplocale_r (struct _reent *p, struct __locale_t *locobj)

> >   {

> > +#ifdef _REENT_SMALL

> > +	return NULL;

> 

> This should set ENOMEM (conceptually, we're so small that ALL attempts to

> create a locale are beyond the memory limits we have available ;)

> 

> By the way, POSIX says duplocale returns '(locale_)0' on failure; you're

> lucky that NULL works here because our locale_t is a wrapper around 'struct

> __locale_t*'.


Why handling this at all?  Does anybody expect a REENT_SMALL
target to actually *call* these functions?  From my POV it would
make more sense to skip compiling these functions into newlib
entirely for SMALL targets.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Patch

===============================  PATCH ==============================
diff --git a/newlib/libc/locale/duplocale.c b/newlib/libc/locale/duplocale.c
index d3e7d782e..7c0108ece 100644
--- a/newlib/libc/locale/duplocale.c
+++ b/newlib/libc/locale/duplocale.c
@@ -42,6 +42,9 @@  PORTABILITY
 struct __locale_t *
 _duplocale_r (struct _reent *p, struct __locale_t *locobj)
 {
+#ifdef _REENT_SMALL
+	return NULL;
+#else
   struct __locale_t tmp_locale, *new_locale;
   int i;
 
@@ -91,6 +94,7 @@  error:
 
   return NULL;
 #endif /* _MB_CAPABLE */
+#endif /* _REENT_SMALL */
 }
 
 #ifndef _REENT_ONLY
diff --git a/newlib/libc/locale/locale.c b/newlib/libc/locale/locale.c
index baa5451a6..ba0654272 100644
--- a/newlib/libc/locale/locale.c
+++ b/newlib/libc/locale/locale.c
@@ -288,6 +288,17 @@  static char *currentlocale (void);
 
 #endif /* _MB_CAPABLE */
 
+#ifdef _REENT_SMALL
+const char *default_decimal_point = ".";
+
+const char *
+loc_get_decimalpoint (void *locale)
+{
+  return default_decimal_point;
+}
+
+#endif
+
 char *
 _setlocale_r (struct _reent *p,
        int category,
@@ -879,7 +890,7 @@  restart:
   switch (category)
     {
     case LC_CTYPE:
-#ifndef __HAVE_LOCALE_INFO__
+#if !defined(__HAVE_LOCALE_INFO__) && !defined (_REENT_SMALL)
       strcpy (loc->ctype_codeset, charset);
       loc->mb_cur_max[0] = mbc_max;
 #endif
@@ -970,10 +981,10 @@  __locale_mb_cur_max (void)
 #ifdef __HAVE_LOCALE_INFO__
   return __get_current_ctype_locale ()->mb_cur_max[0];
 #else
-  return __get_current_locale ()->mb_cur_max[0];
+  return __locale_mb_cur_max_l(__get_current_locale ());
 #endif
 }
-
+#ifndef _REENT_SMALL
 const char *
 __locale_ctype_ptr_l (struct __locale_t *locale)
 {
@@ -985,7 +996,7 @@  __locale_ctype_ptr (void)
 {
   return __get_current_locale ()->ctype_ptr;
 }
-
+#endif
 #ifndef _REENT_ONLY
 
 char *
diff --git a/newlib/libc/locale/localeconv.c
b/newlib/libc/locale/localeconv.c
index 5f34a785f..0e2be2d42 100644
--- a/newlib/libc/locale/localeconv.c
+++ b/newlib/libc/locale/localeconv.c
@@ -2,6 +2,8 @@ 
 #include <reent.h>
 #include "setlocale.h"
 
+#ifndef _REENT_SMALL
+
 struct lconv *
 __localeconv_l (struct __locale_t *locale)
 {
@@ -65,4 +67,8 @@  localeconv (void)
 {
   return __localeconv_l (__get_current_locale ());
 }
+#endif /* _REENT_SMALL */
+
 #endif
+
+
diff --git a/newlib/libc/locale/newlocale.c b/newlib/libc/locale/newlocale.c
index c6c2a9ca9..43e60cbb3 100644
--- a/newlib/libc/locale/newlocale.c
+++ b/newlib/libc/locale/newlocale.c
@@ -86,6 +86,9 @@  struct __locale_t *
 _newlocale_r (struct _reent *p, int category_mask, const char *locale,
 	      struct __locale_t *base)
 {
+#ifdef _REENT_SMALL
+	return NULL;
+#else	
 #ifndef _MB_CAPABLE
   return __get_C_locale ();
 #else /* _MB_CAPABLE */
@@ -213,6 +216,7 @@  error:
 
   return NULL;
 #endif /* _MB_CAPABLE */
+#endif /* _REENT_SMALL */
 }
 
 struct __locale_t *
diff --git a/newlib/libc/locale/nl_langinfo.c
b/newlib/libc/locale/nl_langinfo.c
index eb984912f..26c055eab 100644
--- a/newlib/libc/locale/nl_langinfo.c
+++ b/newlib/libc/locale/nl_langinfo.c
@@ -370,6 +370,7 @@  do_codeset:
 		break;
 #endif
 	default:
+#ifndef _REENT_SMALL		   
 		/* Relies on the fact that LC_ALL is 0, and all other
 		   LC_ constants are in ascending order. */
 		if (item > NL_LOCALE_NAME(LC_ALL)
@@ -377,6 +378,7 @@  do_codeset:
 			return locale->categories[item
 						  - NL_LOCALE_NAME(LC_ALL)];
 		}
+#endif		   
 #ifdef __HAVE_LOCALE_INFO_EXTENDED__
 		if (item > _NL_LOCALE_EXTENDED_FIRST_ENTRY
 		    && item < _NL_LOCALE_EXTENDED_LAST_ENTRY) {
diff --git a/newlib/libc/locale/setlocale.h b/newlib/libc/locale/setlocale.h
index f74aedd45..1b31cad03 100644
--- a/newlib/libc/locale/setlocale.h
+++ b/newlib/libc/locale/setlocale.h
@@ -209,15 +209,23 @@  extern size_t _wcsnrtombs_l (struct _reent *, char *,
const wchar_t **,
 _ELIDABLE_INLINE struct __locale_t *
 __get_global_locale ()
 {
+#ifdef _REENT_SMALL
+  return NULL;
+#else
   extern struct __locale_t __global_locale;
   return &__global_locale;
+#endif
 }
 
 /* Per REENT locale.  This is newlib-internal. */
 _ELIDABLE_INLINE struct __locale_t *
 __get_locale_r (struct _reent *r)
 {
+#ifdef _REENT_SMALL
+  return NULL;
+#else
   return r->_locale;
+#endif	
 }