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

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix overflow at return wchar2char (src/backend/utils/adt/pg_locale.c)
Date: 2020-09-14 13:53:01
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On 14 Sep 2020, at 14:41, Ranier Vilela <ranier(dot)vf(at)gmail(dot)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:

The wcstombs() function returns the number of bytes converted (not
including any terminating null), if successful; otherwise, it returns

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.) 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

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Konstantin Knizhnik 2020-09-14 14:12:42 Re: On login trigger: take three
Previous Message Amit Langote 2020-09-14 13:45:17 Re: problem with RETURNING and update row movement