getting rid of some of the LOCALE magic

[Tobias Oetiker-3]

People,

in rrd_graph I am calling

#ifdef HAVE_SETLOCALE
    setlocale(LC_TIME, "");
#ifdef HAVE_MBSTOWCS
    setlocale(LC_CTYPE, "");
#endif
#endif

this causes the locale settings to be reset to whatever the
coresponding environment variable sais ... this is nice when people
set the LC_* variable in the surrounding code ... but it cause
unexpected behaviour, if people use setlocale when calling rrdtool
as a library ...

I think about dropping the calls for 1.4 ... what do you think ?

cheers
tobi



--
Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland
http://it.oetiker.ch tobi@... ++41 62 775 9902 / sb: -9900

_______________________________________________
rrd-developers mailing list
rrd-developers@...
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers

[Florian Forster-2]

Hi,

On Fri, Sep 04, 2009 at 09:54:37AM +0200, Tobias Oetiker wrote:
> this causes the locale settings to be reset to whatever the
> coresponding environment variable sais ... this is nice when people
> set the LC_* variable in the surrounding code ... but it cause
> unexpected behaviour, if people use setlocale when calling rrdtool as
> a library ...

I'm basically anti-magic. I think the expected behavior would be to
honor the environment when run from the command line and not to change
the existing locale setting when called as a library. I'd therefore move
that code from the “librrd” library to the “rrdtool” application.

Regards,
-octo
--
Florian octo Forster
Hacker in training
GnuPG: 0x91523C3D
http://verplant.org/


_______________________________________________
rrd-developers mailing list
rrd-developers@...
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers

[Benny Baumann-3]

Hi,

The reason for those lines is - I guess - to get a defined language
environment inside the library. I'd more or less correct those to first
query the current env and then set the one the library expects. That way
you avoid some issues with number format conversions especially when
parsing numbers in floating point arguments.

Regards,
BenBE.

Am 04.09.2009 09:54, schrieb Tobias Oetiker:

> People,
>
> in rrd_graph I am calling
>
> #ifdef HAVE_SETLOCALE
>     setlocale(LC_TIME, "");
> #ifdef HAVE_MBSTOWCS
>     setlocale(LC_CTYPE, "");
> #endif
> #endif
>
> this causes the locale settings to be reset to whatever the
> coresponding environment variable sais ... this is nice when people
> set the LC_* variable in the surrounding code ... but it cause
> unexpected behaviour, if people use setlocale when calling rrdtool
> as a library ...
>
> I think about dropping the calls for 1.4 ... what do you think ?
>
> cheers
> tobi
>
>
>
>  

_______________________________________________
rrd-developers mailing list
rrd-developers@...
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers

[Sebastian Harl]

Hi,

On Fri, Sep 04, 2009 at 12:48:25PM +0200, Benny Baumann wrote:

> Am 04.09.2009 09:54, schrieb Tobias Oetiker:
> > in rrd_graph I am calling
> >
> > #ifdef HAVE_SETLOCALE
> >     setlocale(LC_TIME, "");
> > #ifdef HAVE_MBSTOWCS
> >     setlocale(LC_CTYPE, "");
> > #endif
> > #endif
> >
> > this causes the locale settings to be reset to whatever the
> > coresponding environment variable sais ... this is nice when people
> > set the LC_* variable in the surrounding code ... but it cause
> > unexpected behaviour, if people use setlocale when calling rrdtool
> > as a library ...
> >
> > I think about dropping the calls for 1.4 ... what do you think ?

> The reason for those lines is - I guess - to get a defined language
> environment inside the library.

As Tobi mentioned, it sets the locale to whatever has been specified in
the users environment, so the settings inside the library are basically
undefined.

> I'd more or less correct those to first query the current env and then
> set the one the library expects. That way you avoid some issues with
> number format conversions especially when parsing numbers in floating
> point arguments.

Uhm … I'm not sure I understood that correctly. If you override the
locale settings specified by the user, you will _get_ issues when
parsing numbers (in fact, anything that has been passed on the command
line). Or did you mean to parse that prior to changing the settings?

Also, when changing the locale settings, the graph output would no
longer be localized - i.e. month names would be in whatever language has
been specified inside the library.

Did I miss your point?

Anyway - I think removing those lines is probably a good idea but I'd
double-check the reasons why they have been introduced to avoid
reintroducing some issues they were supposed to fix. Has this been
documented, e.g. in some commit message?

Cheers,
Sebastian

--
Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/

Those who would give up Essential Liberty to purchase a little Temporary
Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin



_______________________________________________
rrd-developers mailing list
rrd-developers@...
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers

[Tobias Oetiker-3]

Sep 4 Florian Forster wrote:

> Hi,
>
> On Fri, Sep 04, 2009 at 09:54:37AM +0200, Tobias Oetiker wrote:
> > this causes the locale settings to be reset to whatever the
> > coresponding environment variable sais ... this is nice when people
> > set the LC_* variable in the surrounding code ... but it cause
> > unexpected behaviour, if people use setlocale when calling rrdtool as
> > a library ...
>
> I'm basically anti-magic. I think the expected behavior would be to
> honor the environment when run from the command line and not to change
> the existing locale setting when called as a library. I'd therefore move
> that code from the ?librrd? library to the ?rrdtool? application.

:-) there we already have a setlocale LC_ALL so this should be
taken care off.

cheers
tobi

--
Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland
http://it.oetiker.ch tobi@... ++41 62 775 9902 / sb: -9900

_______________________________________________
rrd-developers mailing list
rrd-developers@...
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers

[Tobias Oetiker-3]

Hi Benny,

Sep 4 Benny Baumann wrote:

> Hi,
>
> The reason for those lines is - I guess - to get a defined language
> environment inside the library. I'd more or less correct those to first
> query the current env and then set the one the library expects. That way
> you avoid some issues with number format conversions especially when
> parsing numbers in floating point arguments.

this is being done for LC_NUMERIC numerous times to avoid getting
bitten by commas instead of . in numbers ... but apart from that
the library should not mess with the locale on its own since it is
written to be able to cope with whatever locale is set ... the
reason these statements were in there was to have the locale set,
when I put them in it was not clear to me that this should be only
done by the main program (as it is now being done by rrd_tool.c).

someone calling the library may also do it before the library is
called and would most likely not be thrilled if the library altered
the settings on the fly to whatever some environemt variable sais
...

cheers
tobi

> Regards,
> BenBE.
>
> Am 04.09.2009 09:54, schrieb Tobias Oetiker:
> > People,
> >
> > in rrd_graph I am calling
> >
> > #ifdef HAVE_SETLOCALE
> >     setlocale(LC_TIME, "");
> > #ifdef HAVE_MBSTOWCS
> >     setlocale(LC_CTYPE, "");
> > #endif
> > #endif
> >
> > this causes the locale settings to be reset to whatever the
> > coresponding environment variable sais ... this is nice when people
> > set the LC_* variable in the surrounding code ... but it cause
> > unexpected behaviour, if people use setlocale when calling rrdtool
> > as a library ...
> >
> > I think about dropping the calls for 1.4 ... what do you think ?
> >
> > cheers
> > tobi
> >
> >
> >
> >
>
>
>

--
Tobi Oetiker, OETIKER+PARTNER AG, Aarweg 15 CH-4600 Olten, Switzerland
http://it.oetiker.ch tobi@... ++41 62 775 9902 / sb: -9900

_______________________________________________
rrd-developers mailing list
rrd-developers@...
https://lists.oetiker.ch/cgi-bin/listinfo/rrd-developers