pgsql: Fix strftime usage on Win32 when trying to fetch the locale-aware

mha@postgresql.org (Magnus Hagander) writes:

Fix strftime usage on Win32 when trying to fetch the locale-aware
parts of a time string so it properly handles different encodings.

Hmm, this patch has

+ 	wchar_t	wbuf[MAX_L10N_DATA];
+ 
+ 	len = wcsftime(wbuf, sizeof(wbuf), format, tm);

The Single Unix Spec's definition of wcsftime says that the above
risks a buffer overrun, and the correct second argument would be
MAX_L10N_DATA. Now I realize that SUS is a poor guide for
Windows-specific code, but are you sure this is right?

Also, I believe we've deprecated StrNCpy; use strlcpy if possible.

regards, tom lane

Tom Lane wrote:

mha@postgresql.org (Magnus Hagander) writes:

Fix strftime usage on Win32 when trying to fetch the locale-aware
parts of a time string so it properly handles different encodings.

Hmm, this patch has

+ 	wchar_t	wbuf[MAX_L10N_DATA];
+ 
+ 	len = wcsftime(wbuf, sizeof(wbuf), format, tm);

The Single Unix Spec's definition of wcsftime says that the above
risks a buffer overrun, and the correct second argument would be
MAX_L10N_DATA. Now I realize that SUS is a poor guide for
Windows-specific code, but are you sure this is right?

Now that I read it again, I think you're right. What MS says is:
"If the total number of characters, including the terminating null, is
more than maxsize, both strftime and wcsftime return 0 and the contents
of strDest are indeterminate."

The important difference being "character" vs "bytes", right?

Also, I believe we've deprecated StrNCpy; use strlcpy if possible.

Ok, I'll change that.

//Magnus

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

The Single Unix Spec's definition of wcsftime says that the above
risks a buffer overrun, and the correct second argument would be
MAX_L10N_DATA. Now I realize that SUS is a poor guide for
Windows-specific code, but are you sure this is right?

Now that I read it again, I think you're right. What MS says is:
"If the total number of characters, including the terminating null, is
more than maxsize, both strftime and wcsftime return 0 and the contents
of strDest are indeterminate."

The important difference being "character" vs "bytes", right?

SUS phrases it as

"If the total number of resulting wide-character codes including the
terminating null wide-character code is no more than maxsize, wcsftime()
returns the number of wide-character codes placed into the array pointed
to by wcs, not including the terminating null wide-character
code. Otherwise 0 is returned and the contents of the array are
indeterminate."

so it's very clear that maxsize is counted in wchars.

Perhaps someone could experiment to double-check what Windows does.

regards, tom lane

Tom Lane wrote:

Magnus Hagander <magnus@hagander.net> writes:

Tom Lane wrote:

The Single Unix Spec's definition of wcsftime says that the above
risks a buffer overrun, and the correct second argument would be
MAX_L10N_DATA. Now I realize that SUS is a poor guide for
Windows-specific code, but are you sure this is right?

Now that I read it again, I think you're right. What MS says is:
"If the total number of characters, including the terminating null, is
more than maxsize, both strftime and wcsftime return 0 and the contents
of strDest are indeterminate."

The important difference being "character" vs "bytes", right?

SUS phrases it as

"If the total number of resulting wide-character codes including the
terminating null wide-character code is no more than maxsize, wcsftime()
returns the number of wide-character codes placed into the array pointed
to by wcs, not including the terminating null wide-character
code. Otherwise 0 is returned and the contents of the array are
indeterminate."

so it's very clear that maxsize is counted in wchars.

Perhaps someone could experiment to double-check what Windows does.

Read up a bit more and compared, it definitely seems to mean the same
thing. My tests seem to agree as well.

I'll change it to MAX_L10N_DATA and strlcpy.

//Magnus