Skip to Content.
Sympa Menu

freetds - Re: [freetds] [PATCH] tds_get_locale() update (was Re: SQL Server version and TDS Version)

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: jklowden AT freetds.org
  • To: FreeTDS Development Group <freetds AT lists.ibiblio.org>
  • Subject: Re: [freetds] [PATCH] tds_get_locale() update (was Re: SQL Server version and TDS Version)
  • Date: Mon, 24 May 2010 08:11:19 -0400

On Sun, May 23, 2010 at 06:13:44PM -0500, Craig A. Berry wrote:
>
> On Apr 30, 2010, at 3:30 PM, jklowden AT schemamania.org wrote:
>
> >On Fri, Apr 30, 2010 at 02:58:12PM -0400, jklowden AT schemamania.org
> >wrote:
> >>
> >>1. API call e.g. DBSETLCHARSET().
> >>2. freetds.conf.
> >>3. nl_langinfo(3) /* query using CODESET parameter */
> >>4. setlocale(3) /* query using NULL parameter, split on "." */
> >>5. ISO 8859-1

> Finally got back to this. The attached patch does two things to src/
> tds/locale.c:tds_get_locale():

[excellent summary omitted]

Hi Craig,

You're looking for a little guidance, which I'll try to provide. I haven't
verified your analysis because 1) it sounds right and 2) if it's not I'm
confident you'll get it right in the end. (And 3: that would take too long.)

> But late in the day I realized that while this tiny refactor is
> probably a step in the right direction, it doesn't really address how
> the client charset is selected because the value determined from the
> environment in tds_alloc_locale() will always be overridden with a
> hard-wired ISO-8859-1 in tds_alloc_connection(). This is because TDS
> applications call tds_alloc_context() before they call
> tds_read_config_info(). The following calling sequences show how we
> end up getting what's done in tds_alloc_connection():
>
> a.) tds_alloc_locale*
> tds_get_locale
> tds_alloc_context
>
> b.) tds_alloc_connection*
> tds_read_config_info
>
> and what happens in either of these can be overridden from
> freetds.conf because the following happens later:
>
> c.) tds_try_conf_file
> tds_read_conf_file
> tds_read_config_info
>
>
> The problem in step b above is that tds_alloc_connection() has the
> following code in it:
>
> /* TODO use system default ?? */
> if (!tds_dstr_copy(&connection->client_charset, "ISO-8859-1"))
> goto Cleanup;
>
> There is no checking to see if the client charset is already set in
> the locale structure, so in effect, locale->client_charset is an
> appendage that is never used for anything. It's already been orphaned
> so we might as well deprecate it.

That sounds right.

> So the next step is to move the nl_langinfo(CODESET) call out of
> tds_alloc_locale() and into tds_alloc_connection(), replacing the hard-
> wired ISO-8859-1 with what we get by interrogating the locale.

Yes, leaving ISO 8859-1 as a fallback if nl_langinfo/setlocale isn't
available.

> But I think that needs more discussion because anyone with a
> misconfigured locale will suddenly not be able to construct a valid
> login packet; basically we'll be doing at the library level what tsql
> has been doing for a long time at the application level, and which
> generated the problem report that led us down this road in the first
> place.

You lost me there. tsql isn't most applications; most applications use one
of the client libraries, which *should* call these things in the same order
that tsql does. If tsql is effectively broken, we can fix it.

I must be missing something, because "anyone with a misconfigured locale will
suddenly not be able to construct a valid login packet" shouldn't be so, for
reasons I'm sure you know full well: anyone with a misconfigured locale can
resort to the freetds.conf override (level #2 above). IOW, once step B is
fixed, step C still happens. So, what's the problem again? :-)

It's great you're working on this. Would you like to be able to commit these
things yourself? No good deed goes unpunished, you know.

Regards,

--jkl




Archive powered by MHonArc 2.6.24.

Top of Page