[Home]

Summary:ASTERISK-13450: [patch] add option to configure locale for date/time string construction
Reporter:klaus3000 (klaus3000)Labels:
Date Opened:2009-01-26 07:35:58.000-0600Date Closed:2010-08-02 15:51:54
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail/NewFeature
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20090515__issue14333.diff.txt
( 1) app_voicemail.c-svn-trunk-rev211675-patch.txt
Description:Hi!

When voicemail sends emails it constructs the date/time when the voicemessage was received. This patch allows to configure the locale for generation of the string.

locale=de_AT.UTF-8 results in

"Montag, 26 Jänner 2009" instead of "Monday, 26 January 2009".

This could also be achieved setting the default OS locale - but this would also change the locale of all other date/time strings in Asterisk (logmesage, CDRs...) which can be bad.
Comments:By: Tilghman Lesher (tilghman) 2009-01-26 17:18:41.000-0600

Unfortunately, this is not going to work.  The problem is that setlocale(3) does not work on a thread-level to influence only the thread's environment.  Instead, it influences the entire program, so for the period that you are using the alternative locale, all other messages (like log messages) will also have the alternative locale.

HOWEVER, there is a new specification in one of the latest POSIX specifications (IEEE Std 1003.1-2008), that has a thread-specific locale, which is exactly what you want.  The API calls are newlocale(3), uselocale(3), and freelocale(3).  You can view the specifications here:  http://www.opengroup.org/onlinepubs/9699919799/basedefs/locale.h.html  If you update to these API calls, then this can be added.
By: klaus3000 (klaus3000) 2009-01-27 13:05:15.000-0600

You are right. I will update it.

btw: apps/app_minivm.c needs to fixed too
By: klaus3000 (klaus3000) 2009-01-29 02:07:39.000-0600

I have ported my new patch (using newlocale) to trunk. Meanwhile I found out that trunk has an ast_strftime function. Maybe it would be better to extend these function with an additional parameter "char *locale" which can be used by all modules. Of course this requires to call the newlocale() function for every request whereas having it in voicemail.c allows to call newlocale() only once during module load.
By: Tilghman Lesher (tilghman) 2009-01-31 10:43:41.000-0600

klaus3000:  That's a good idea.  And actually, you don't need to deallocate the locale every time in ast_strftime(); simply store the locale information in a linked list, and you can retrieve the appropriate locale each time, by comparing the string used.
By: klaus3000 (klaus3000) 2009-02-02 05:59:15.000-0600

Not sure what you mean with linked list. I would write an ast_strftime_locale(....., locale_t locale) function which sets/resets the locale before/after strftime.

The locale will be created by the module (e.g. app_voicemail) during module load.

So, what for is the linked list?

Further, I  have not fully understand the concept of freeing the memory (bad documentation). resetting the locale with uselocale(LC_GLOBAL_LOCALE) returns a locale which holds the previous locale. My tests showed that uselocale points to the same locale multiple times - thus freeing this memory may cause problem when it is freed in mutliple modules.
By: pkempgen (pkempgen) 2009-02-02 06:08:43.000-0600

> Not sure what you mean with linked list.
http://en.wikipedia.org/wiki/Linked_list
By: klaus3000 (klaus3000) 2009-02-02 06:39:10.000-0600

sorry, wrong words ...

Yes, I know what a linked list is. But I do not understand how/why a local list should be used to store the locales.

Do you want to configure locales globaly and in the modules refering to these global modules?
By: Tilghman Lesher (tilghman) 2009-02-02 08:00:05.000-0600

Please see the linked list currently used for timezones and do the same thing with locales.
By: klaus3000 (klaus3000) 2009-02-02 09:33:48.000-0600

do you mean that the locale should be per user defined?
By: Tilghman Lesher (tilghman) 2009-02-02 12:46:54.000-0600

I figured it was probably easier just to write it than to explain the concept.
By: klaus3000 (klaus3000) 2009-03-12 10:48:36

Hi! I do not have time to implement this. Unless you want to fix it anyway feel free to close the bug.

Maybe somebody wants to do a Janitor project - I think this one would be a good starter for Asterisk coding.
By: Tilghman Lesher (tilghman) 2009-03-12 10:54:20

At this point, implementation is a moot point.  All that this needs right now is testing.
By: klaus3000 (klaus3000) 2009-03-20 05:19:47

ups. sorry - I have not seen that you have added a patch. I will test it.
By: Tilghman Lesher (tilghman) 2009-04-03 15:41:50

klaus3000: have you made any progress on testing this?
By: klaus3000 (klaus3000) 2009-04-04 03:40:11

Not yet. I wanted to try it now, but found out, that this is not a complete feature patch for the voicemail application, but just the extension to localtime.c.

Now I wonder how to use the framework correctly. If I understand it correctly other modules have 2 choices:

1. prev=ast_setlocale(new)
2. ast_strftime(...)
3. ast_setlocale(prev)

or just use
  ast_strftime_locale(...., new)

Is this correct?
By: klaus3000 (klaus3000) 2009-04-04 03:49:08

building (trunk) fails:
  [CC] ael_main.c -> ael_main.o
In file included from /export/darilion/asterisk-1.4-versionen/svn-trunk/include/asterisk/utils.h:34,
                from /export/darilion/asterisk-1.4-versionen/svn-trunk/include/asterisk/config.h:30,
                from /export/darilion/asterisk-1.4-versionen/svn-trunk/include/asterisk/channel.h:143,
                from ael_main.c:18:
/export/darilion/asterisk-1.4-versionen/svn-trunk/include/asterisk/localtime.h:30: error: conflicting types for ‘locale_t’
/usr/include/locale.h:148: error: previous declaration of ‘locale_t’ was here
make[1]: *** [ael_main.o] Error 1
By: Tilghman Lesher (tilghman) 2009-04-04 11:18:32

No, the way to use this would be just the second case, and you don't use the locale setting, merely pass the locale name.  The caching is done transparently.
By: klaus3000 (klaus3000) 2009-04-04 15:46:42

ok. and what about the build failure?
By: Tilghman Lesher (tilghman) 2009-05-07 14:03:02

New patch uploaded, ready for testing.
By: klaus3000 (klaus3000) 2009-05-08 09:22:54

Thanks! I'm currently traveling without access to my test server. So it will last till June for testing, sorry.
By: Tilghman Lesher (tilghman) 2009-08-11 12:25:19

klaus3000:  it is now August.  Have you had a chance to test this yet?
By: klaus3000 (klaus3000) 2009-08-12 09:08:00

tilghman, I modyfied app_voicemail to make use of the new functions and it works (see uploaded patch).

But I had problems building Asterisk with support for newlocale - autodetection did not work. I had to define manually "#define HAVE_NEWLOCALE 1" in autoconfigure.h to enable the feature.
By: klaus3000 (klaus3000) 2009-10-30 04:49:12

I think you can remove all but the last 2 patches, and make it ready for testing?
By: Digium Subversion (svnbot) 2010-06-01 16:28:19

Repository: asterisk
Revision: 266828

U   trunk/CHANGES
U   trunk/apps/app_voicemail.c
U   trunk/configs/voicemail.conf.sample
U   trunk/configure
U   trunk/configure.ac
U   trunk/include/asterisk/autoconfig.h.in
U   trunk/include/asterisk/localtime.h
U   trunk/main/stdtime/localtime.c
A   trunk/tests/test_locale.c

------------------------------------------------------------------------
r266828 | tilghman | 2010-06-01 16:28:19 -0500 (Tue, 01 Jun 2010) | 9 lines

Support setting locale per-mailbox (changes date/time languages for email, pager messages).

(closes issue ASTERISK-13450)
Reported by: klaus3000
Patches:
      20090515__issue14333.diff.txt uploaded by tilghman (license 14)
      app_voicemail.c-svn-trunk-rev211675-patch.txt uploaded by klaus3000 (license 65)
Tested by: klaus3000

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=266828
By: Jeffrey C. Ollie (jcollie) 2010-08-02 10:53:35

I'm having problems compiling chan_misdn, and I think that they may be related to this change:

gcc -o misdn/isdn_msg_parser.o -c misdn/isdn_msg_parser.c -MD -MT misdn/isdn_msg_parser.o -MF .misdn_isdn_msg_parser.o.d -MP -
pthread -I/builddir/build/BUILD/asterisk-1.8.0-beta2/include -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-p
rotector --param=ssp-buffer-size=4 -m64 -mtune=generic -Werror-implicit-function-declaration  -I/usr/include/libxml2 -Wstrict-
prototypes -Wmissing-prototypes -Wmissing-declarations  -march=k8   -fPIC -DAST_MODULE=\"chan_misdn\"     -O2 -g -pipe -Wall -
Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -Werror-implicit-function-
declaration
In file included from misdn/ie.c:34:0,
                from misdn/isdn_msg_parser.c:26:
/builddir/build/BUILD/asterisk-1.8.0-beta2/include/asterisk/localtime.h:32:16: error: conflicting types for 'locale_t'
/usr/include/xlocale.h:43:20: note: previous declaration of 'locale_t' was here
By: Jeffrey C. Ollie (jcollie) 2010-08-02 11:04:20

Disabling the build of chan_misdn allows the compile to succeed.  This is on Fedora 13 and Rawhide
By: Tilghman Lesher (tilghman) 2010-08-02 15:45:05

Possible, but please open a new issue, relating the change to this one, instead of reopening this old issue.