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

On Apr 30, 2010, at 3:30 PM, [email protected] wrote:

> On Fri, Apr 30, 2010 at 02:58:12PM -0400, [email protected]  
> 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:[email protected]

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

_______________________________________________
FreeTDS mailing list
[email protected]
http://lists.ibiblio.org/mailman/listinfo/freetds

Index: src/tds/locale.c
===================================================================
RCS file: /cvsroot/freetds/freetds/src/tds/locale.c,v
retrieving revision 1.28
diff -p -u -r1.28 locale.c
--- src/tds/locale.c	16 Oct 2007 15:12:22 -0000	1.28
+++ src/tds/locale.c	23 May 2010 03:05:26 -0000
@@ -22,6 +22,10 @@
 #include <config.h>
 #endif
 
+#if HAVE_LOCALE_H
+#include <locale.h>
+#endif
+
 #include <stdarg.h>
 #include <stdio.h>
 #include <ctype.h>
@@ -68,12 +72,16 @@ tds_get_locale(void)
 	if (in) {
 		tds_read_conf_section(in, "default", tds_parse_locale, locale);
 
+#if HAVE_LOCALE_H
+		setlocale(LC_ALL, "");
+		s = setlocale(LC_ALL, NULL);
+#else
 		s = getenv("LANG");
+#endif
 		if (s && s[0]) {
 			int found;
 			char buf[128];
 			const char *strip = "@._";
-			const char *charset = NULL;
 
 			/* do not change environment !!! */
 			tds_strlcpy(buf, s, sizeof(buf));
@@ -96,19 +104,13 @@ tds_get_locale(void)
 				if (!s)
 					continue;
 				*s = 0;
-				if (*strip == '.')
-					charset = s+1;
 				rewind(in);
 				found = tds_read_conf_section(in, buf, tds_parse_locale, locale);
 			}
 
-			/* charset specified in LANG ?? */
-			if (charset) {
-				free(locale->client_charset);
-				locale->client_charset = strdup(charset);
-			}
 		}
 
+
 		fclose(in);
 	}
 	return locale;