Message-ID: <492AC031.7010901@hagander.net>
Date: Mon, 24 Nov 2008 15:54:41 +0100
From: Magnus Hagander <magnus@hagander.net>
User-Agent: Thunderbird 2.0.0.17 (X11/20080925)
MIME-Version: 1.0
To: Tom Lane <tgl@sss.pgh.pa.us>
CC: Hiroshi Saito <z-saito@guitar.ocn.ne.jp>,
	ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>,
	Jaime Casanova <jcasanov@systemguards.com.ec>,
	Alvaro Herrera <alvherre@commandprompt.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCHES] Solve a problem of LC_TIME of windows.
References: <20081104094301.7EE8.52131E4D@oss.ntt.co.jp>
	<3073cc9b0811102212n278644a8o7169f7c5ea562081@mail.gmail.com>
	<20081111152112.B605.52131E4D@oss.ntt.co.jp>
	<A32F4AB7D95646B3A358002E1FB73F9C@HIRO57887DE653>
	<446F4763B989489C9B626D21AB9F6C7F@HIRO57887DE653>
	<492AA5CD.8050001@hagander.net> <18269.1227538234@sss.pgh.pa.us>
In-Reply-To: <18269.1227538234@sss.pgh.pa.us>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Tom Lane wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> *** a/src/backend/utils/adt/pg_locale.c
>> --- b/src/backend/utils/adt/pg_locale.c
>> ***************
>> *** 54,59 ****
>> --- 54,60 ----
>>   #include "utils/memutils.h"
>>   #include "utils/pg_locale.h"
>   
>> + #include "mb/pg_wchar.h"
>   
>>   #define		MAX_L10N_DATA		80
> 
> Please stick to the convention of including include files in
> alphabetical order.

Check.


>> + strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm *tm)
>> + {
>> + 	size_t	len;
>> + 	wchar_t	wbuf[MAX_L10N_DATA];
>> + 	int		encoding;
>> + 
>> + 	encoding = GetDatabaseEncoding();
>> + 	if (encoding == PG_SQL_ASCII)
>> + 		return len;
> 
> Surely this is returning an uninitialized variable, not to mention
> failing to accomplish any of the goals of the function.  I don't think
> breaking things completely for SQL_ASCII was part of the plan.

Gah, true, that's me breaking it. That was correct in Hiroshi-san's
patch. My bad, sorry.


> 
>> + 		ereport(ERROR,
>> + 			(errmsg("could not convert string to UTF-8:error %lu", GetLastError())));
> 
> This is not exactly per message style guidelines.  Maybe it's just a
> can't-happen case, but if so make it elog not ereport.

Check.

//Magnus