Message ID | 20170619161817.17415402AEC0E@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
On 19/06/2017 13:18, Florian Weimer wrote: > 2017-06-19 Florian Weimer <fweimer@redhat.com> > > * intl/dcigettext.c: Remove alloca support. > (DCIGETTEXT): Use heap allocations for xdomainname, single_locale. I think this could be another possible usage of char_array, but to safely use it would require to slight change '_nl_find_domain' to avoid tamper the 'locale' argument and making a copy otherwise (current approach seems a bad idiom imho, where it allocates a buffer in some cases and redirect to input argument). So I am not sure if it worth the trouble, the only meaningful in this case is a slight better performance due less heap usage for general cases. So patch LGTM, but I can send a variant which uses char_array if you prefer as well. > > diff --git a/intl/dcigettext.c b/intl/dcigettext.c > index 0e79b1f..d778d19 100644 > --- a/intl/dcigettext.c > +++ b/intl/dcigettext.c > @@ -27,28 +27,6 @@ > > #include <sys/types.h> > > -#ifdef __GNUC__ > -# define alloca __builtin_alloca > -# define HAVE_ALLOCA 1 > -#else > -# ifdef _MSC_VER > -# include <malloc.h> > -# define alloca _alloca > -# else > -# if defined HAVE_ALLOCA_H || defined _LIBC > -# include <alloca.h> > -# else > -# ifdef _AIX > - #pragma alloca > -# else > -# ifndef alloca > -char *alloca (); > -# endif > -# endif > -# endif > -# endif > -#endif > - > #include <errno.h> > #ifndef errno > extern int errno; > @@ -374,45 +352,6 @@ static const char *get_output_charset (struct binding *domainbinding) > #endif > > > -/* For those losing systems which don't have `alloca' we have to add > - some additional code emulating it. */ > -#ifdef HAVE_ALLOCA > -/* Nothing has to be done. */ > -# define freea(p) /* nothing */ > -# define ADD_BLOCK(list, address) /* nothing */ > -# define FREE_BLOCKS(list) /* nothing */ > -#else > -struct block_list > -{ > - void *address; > - struct block_list *next; > -}; > -# define ADD_BLOCK(list, addr) \ > - do { \ > - struct block_list *newp = (struct block_list *) malloc (sizeof (*newp)); \ > - /* If we cannot get a free block we cannot add the new element to \ > - the list. */ \ > - if (newp != NULL) { \ > - newp->address = (addr); \ > - newp->next = (list); \ > - (list) = newp; \ > - } \ > - } while (0) > -# define FREE_BLOCKS(list) \ > - do { \ > - while (list != NULL) { \ > - struct block_list *old = list; \ > - list = list->next; \ > - free (old->address); \ > - free (old); \ > - } \ > - } while (0) > -# undef alloca > -# define alloca(size) (malloc (size)) > -# define freea(p) free (p) > -#endif /* have alloca */ > - > - > #ifdef _LIBC > /* List of blocks allocated for translations. */ > typedef struct transmem_list > @@ -488,17 +427,14 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, > int plural, unsigned long int n, int category) > #endif > { > -#ifndef HAVE_ALLOCA > - struct block_list *block_list = NULL; > -#endif > struct loaded_l10nfile *domain; > struct binding *binding; > const char *categoryname; > const char *categoryvalue; > const char *dirname; > char *xdirname = NULL; > - char *xdomainname; > - char *single_locale; > + char *xdomainname = NULL; > + char *single_locale = NULL; > char *retval; > size_t retlen; > int saved_errno; > @@ -507,7 +443,6 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, > #if defined HAVE_PER_THREAD_LOCALE && !defined IN_LIBGLOCALE > const char *localename; > #endif > - size_t domainname_len; > > /* If no real MSGID is given return NULL. */ > if (msgid1 == NULL) > @@ -537,6 +472,7 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, > definition left this undefined. */ > if (domainname == NULL) > domainname = _nl_current_default_domain; > + size_t domainname_len = strlen (domainname); > > /* OS/2 specific: backward compatibility with older libintl versions */ > #ifdef LC_MESSAGES_COMPAT > @@ -652,19 +588,16 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, > categoryvalue = guess_category_value (category, categoryname); > #endif > > - domainname_len = strlen (domainname); > - xdomainname = (char *) alloca (strlen (categoryname) > - + domainname_len + 5); > - ADD_BLOCK (block_list, xdomainname); > - > - stpcpy ((char *) mempcpy (stpcpy (stpcpy (xdomainname, categoryname), "/"), > - domainname, domainname_len), > - ".mo"); > - > /* Creating working area. */ > - single_locale = (char *) alloca (strlen (categoryvalue) + 1); > - ADD_BLOCK (block_list, single_locale); > + single_locale = malloc (strlen (categoryvalue) + 1); > > + if (single_locale == NULL > + || __asprintf (&xdomainname, "%s/%s.mo", categoryname, domainname) < 0) > + { > + free (single_locale); > + free (xdirname); > + return NULL; > + } > > /* Search for the given string. This is a loop because we perhaps > got an ordered list of languages to consider for the translation. */ > @@ -752,7 +685,8 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, > /* Found the translation of MSGID1 in domain DOMAIN: > starting at RETVAL, RETLEN bytes. */ > free (xdirname); > - FREE_BLOCKS (block_list); > + free (xdomainname); > + free (single_locale); > if (foundp == NULL) > { > /* Create a new entry and add it to the search tree. */ > @@ -836,7 +770,8 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, > return_untranslated: > /* Return the untranslated MSGID. */ > free (xdirname); > - FREE_BLOCKS (block_list); > + free (xdomainname); > + free (single_locale); > gl_rwlock_unlock (_nl_state_lock); > #ifdef _LIBC > __libc_rwlock_unlock (__libc_setlocale_lock); >
diff --git a/intl/dcigettext.c b/intl/dcigettext.c index 0e79b1f..d778d19 100644 --- a/intl/dcigettext.c +++ b/intl/dcigettext.c @@ -27,28 +27,6 @@ #include <sys/types.h> -#ifdef __GNUC__ -# define alloca __builtin_alloca -# define HAVE_ALLOCA 1 -#else -# ifdef _MSC_VER -# include <malloc.h> -# define alloca _alloca -# else -# if defined HAVE_ALLOCA_H || defined _LIBC -# include <alloca.h> -# else -# ifdef _AIX - #pragma alloca -# else -# ifndef alloca -char *alloca (); -# endif -# endif -# endif -# endif -#endif - #include <errno.h> #ifndef errno extern int errno; @@ -374,45 +352,6 @@ static const char *get_output_charset (struct binding *domainbinding) #endif -/* For those losing systems which don't have `alloca' we have to add - some additional code emulating it. */ -#ifdef HAVE_ALLOCA -/* Nothing has to be done. */ -# define freea(p) /* nothing */ -# define ADD_BLOCK(list, address) /* nothing */ -# define FREE_BLOCKS(list) /* nothing */ -#else -struct block_list -{ - void *address; - struct block_list *next; -}; -# define ADD_BLOCK(list, addr) \ - do { \ - struct block_list *newp = (struct block_list *) malloc (sizeof (*newp)); \ - /* If we cannot get a free block we cannot add the new element to \ - the list. */ \ - if (newp != NULL) { \ - newp->address = (addr); \ - newp->next = (list); \ - (list) = newp; \ - } \ - } while (0) -# define FREE_BLOCKS(list) \ - do { \ - while (list != NULL) { \ - struct block_list *old = list; \ - list = list->next; \ - free (old->address); \ - free (old); \ - } \ - } while (0) -# undef alloca -# define alloca(size) (malloc (size)) -# define freea(p) free (p) -#endif /* have alloca */ - - #ifdef _LIBC /* List of blocks allocated for translations. */ typedef struct transmem_list @@ -488,17 +427,14 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, int plural, unsigned long int n, int category) #endif { -#ifndef HAVE_ALLOCA - struct block_list *block_list = NULL; -#endif struct loaded_l10nfile *domain; struct binding *binding; const char *categoryname; const char *categoryvalue; const char *dirname; char *xdirname = NULL; - char *xdomainname; - char *single_locale; + char *xdomainname = NULL; + char *single_locale = NULL; char *retval; size_t retlen; int saved_errno; @@ -507,7 +443,6 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, #if defined HAVE_PER_THREAD_LOCALE && !defined IN_LIBGLOCALE const char *localename; #endif - size_t domainname_len; /* If no real MSGID is given return NULL. */ if (msgid1 == NULL) @@ -537,6 +472,7 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, definition left this undefined. */ if (domainname == NULL) domainname = _nl_current_default_domain; + size_t domainname_len = strlen (domainname); /* OS/2 specific: backward compatibility with older libintl versions */ #ifdef LC_MESSAGES_COMPAT @@ -652,19 +588,16 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, categoryvalue = guess_category_value (category, categoryname); #endif - domainname_len = strlen (domainname); - xdomainname = (char *) alloca (strlen (categoryname) - + domainname_len + 5); - ADD_BLOCK (block_list, xdomainname); - - stpcpy ((char *) mempcpy (stpcpy (stpcpy (xdomainname, categoryname), "/"), - domainname, domainname_len), - ".mo"); - /* Creating working area. */ - single_locale = (char *) alloca (strlen (categoryvalue) + 1); - ADD_BLOCK (block_list, single_locale); + single_locale = malloc (strlen (categoryvalue) + 1); + if (single_locale == NULL + || __asprintf (&xdomainname, "%s/%s.mo", categoryname, domainname) < 0) + { + free (single_locale); + free (xdirname); + return NULL; + } /* Search for the given string. This is a loop because we perhaps got an ordered list of languages to consider for the translation. */ @@ -752,7 +685,8 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, /* Found the translation of MSGID1 in domain DOMAIN: starting at RETVAL, RETLEN bytes. */ free (xdirname); - FREE_BLOCKS (block_list); + free (xdomainname); + free (single_locale); if (foundp == NULL) { /* Create a new entry and add it to the search tree. */ @@ -836,7 +770,8 @@ DCIGETTEXT (const char *domainname, const char *msgid1, const char *msgid2, return_untranslated: /* Return the untranslated MSGID. */ free (xdirname); - FREE_BLOCKS (block_list); + free (xdomainname); + free (single_locale); gl_rwlock_unlock (_nl_state_lock); #ifdef _LIBC __libc_rwlock_unlock (__libc_setlocale_lock);