> 1. wchar2char has a mistake when checking the return of WideCharToMultiByte call. > result variable is unsigned, therefore, cannot be less than zero, returning -1 is not an option.
If the objection is that an unsigned var is tested with <= 0, then changing the semantics of the function seems a rather drastic solution:
/* A zero return is failure */ - if (result <= 0) - result = -1; + if (result == 0) + return 0;
The comment for wchar2char explicitly state "This has the same API as the standard wcstombs_l() function;", and man wcstombs_l shows:
RETURN VALUES The wcstombs() function returns the number of bytes converted (not including any terminating null), if successful; otherwise, it returns (size_t)-1.
I'm not agree that must, follow wcstombs_l API.
And there is already a precedent in returning zero.
But if wchar2char must be follow wcstombs_l API.
wchar2char all calls must be:
result = wchar2char();
if (result == 0 || result == (size_t)-1) {
See at lowerstr_with_len (src/backend/tsearch/ts_locale.c):
wlen = char2wchar(wstr, len + 1, str, len, mylocale); // fail with -1
len = pg_database_encoding_max_length() * 18446744073709551615 + 1; // len is int, Windows LLP64 is 32 bits.
out = (char *) palloc(len);
> 2. strftime or strftime_win32, return cannot be less than zero. > > 3. If strftime or strftime_win32, fails, why not abort the loop?
This recently changed in 7ad1cd31bfc, and the commit message along with the comment above the code implies that an error is unlikely:,
* MAX_L10N_DATA is sufficient buffer space for every known locale, and * POSIX defines no strftime() errors. (Buffer space exhaustion is not an * error.)
..so it's probably a case of not optimizing for never-happens-scenarios: The fact that strftimefail will trigger elog and not ereport is an additional clue that an error is unlikely.
The cost of the test, It has been paid, then the break is free.
And testing unsigned if it is less than zero, it is useless,
