Skip to Content.
Sympa Menu

freetds - [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: "Craig A. Berry" <craigberry AT mac.com>
  • To: FreeTDS Development Group <freetds AT lists.ibiblio.org>
  • Subject: [freetds] [PATCH] tds_get_locale() update (was Re: SQL Server version and TDS Version)
  • Date: Sun, 23 May 2010 18:13:44 -0500


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


I committed changes to tds_alloc_locale()
to get its initial values as I described.

The search-for-canonical has to stay, but the parsing of LANG has to go.
tds_get_locale() looks mostly wrong, but it's a little hard to untangle.
All that really has to happen is to canonicalize the value returned by
tds_alloc_locale() and overwrite that with whatever is read from
the configuration files.

(The setlocale(3) stuff arguably should be in tds_get_locale(). I wouldn't
argue if it got moved there.)


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

1.) Parses the output of setlocale() rather than the value of LANG. This should almost always result in the same answer, but seems like the more canonical way to do it. The one case where behavior will be different is when LANG is unset: we'll end up searching locales.conf for the "C" locale. This shouldn't do any harm but probably not much good either. I suppose we could make [C] an alias for [default] or give it some other kind of special handling if anyone thought it was necessary.

2.) Divorces client charset handling from the processing of locales.conf. This is consistent with the documentation for locales.conf, which says, "If your purpose is to affect the client charset description, use freetds.conf instead." I think it also follows naturally from James's patch of 30 April that uses nl_langinfo() in tds_alloc_locale to identify a default client character set. Parsing the client charset name out of the locale name should give us the same answer if we're doing it right but is overly complex and unnecessary and creates another place where we could get it wrong.

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.

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.

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.

________________________________________
Craig A. Berry
mailto:craigberry AT mac.com

"... getting out of a sonnet is much more
difficult than getting in."
Brad Leithauser

Attachment: tds_get_locale.patch
Description: Binary data




Archive powered by MHonArc 2.6.24.

Top of Page