Re: Fix overflow at return wchar2char (src/backend/utils/adt/pg_locale.c)

> 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