Re: Avoid a possible null pointer (src/backend/utils/adt/pg_locale.c)

On Sun, Sep 10, 2023 at 06:28:08PM -0300, Ranier Vilela wrote:
> +1
> But as Jeff mentioned, when the locale is NULL,
> the provider is known to be COLLPROVIDER_LIBC.
>
> I think we can use this to provide an error message,
> when the locale is NULL.
>
> What do you think about v3 attached?

This does not apply for me on HEAD, and it seems to me that the patch
has some parts that apply on top of v2 (or v1?) while others would
apply to HEAD.

Anyway, what you are suggesting to change compared to v2 is that:

+ /*
+ * if locale is NULL, then
+ * the provider is known to be COLLPROVIDER_LIBC
+ */
if (!locale)
- elog(ERROR, "unsupported collprovider");
+ elog(ERROR, "collprovider '%c' does not support (%s)",
+ COLLPROVIDER_LIBC, "pg_strxfrm_prefix");

I'm OK with enforcing COLLPROVIDER_LIBC in this path, but I also value
consistency across all the error messages of this file. After
sleeping on it, and as that's a set of elogs, "unsupported
collprovider" is fine for me across the board as these should not be
user-visible.

This should be made consistent down to 16, I guess, but only after
16.0 is tagged as we are in release week.
--
Michael