> On 14 Sep 2020, at 14:41, Ranier Vilela <ranier.vf@gmail.com> wrote:
> 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.
It can of course be argued that the check should be "result == 0" as result is
of type size_t. The original commit introducing this in 2007, 654dcfb9e4b6,
had an integer return variable, so it's just a carry-over from there. Will
changing that buy us anything, except possibly silence a static analyzer?
> 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.
cheers ./daniel
