Hi all,
This is a follow-up of
https://www.postgresql.org/message-id/11202.1472597262%40sss.pgh.pa.us
where we are looking at improving OOM handling in the code. In short,
report an ERROR appropriately instead of crashing. As mentioned in
this message, pg_locale.c is trickier to handle because we had better
not call elog() in a code path where the backend's locale are not set
up appropriately. Attached is a patch aimed at fixing that, doing the
following:
- Copy into a temporary struct lconv the results from the call of
localeconv() as those can be overwritten when restoring back the
locales with setlocale().
- Use db_encoding_strdup to encode that correctly.
- Switch back to the backend locales
- Check for any strdup calls that returned NULL and elog()
- If no error, fill in CurrentLocaleConv and return back to caller.
I am attaching that to the next CF.
Thanks,
--
Michael
diff --git a/src/backend/utils/adt/pg_locale.c
b/src/backend/utils/adt/pg_locale.c
index a818023..e4fd0f3 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -440,6 +440,7 @@ PGLC_localeconv(void)
static struct lconv CurrentLocaleConv;
static bool CurrentLocaleConvAllocated = false;
struct lconv *extlconv;
+ struct lconv tmplconv;
char *save_lc_monetary;
char *save_lc_numeric;
char *decimal_point;
@@ -523,23 +524,22 @@ PGLC_localeconv(void)
encoding = pg_get_encoding_from_locale(locale_monetary, true);
/*
- * Must copy all values since restoring internal settings may overwrite
- * localeconv()'s results. Note that if we were to fail within this
- * sequence before reaching "CurrentLocaleConvAllocated = true", we
could
- * leak some memory --- but not much, so it's not worth agonizing over.
+ * First copy all values into a temporary variable since restoring
internal
+ * settings may overwrite localeconv()'s results. Note that elog()
cannot
+ * be called as the backend's locale settings prevail and they are not
+ * restored yet.
*/
- CurrentLocaleConv = *extlconv;
- CurrentLocaleConv.decimal_point = decimal_point;
- CurrentLocaleConv.grouping = grouping;
- CurrentLocaleConv.thousands_sep = thousands_sep;
- CurrentLocaleConv.int_curr_symbol = db_encoding_strdup(encoding,
extlconv->int_curr_symbol);
- CurrentLocaleConv.currency_symbol = db_encoding_strdup(encoding,
extlconv->currency_symbol);
- CurrentLocaleConv.mon_decimal_point = db_encoding_strdup(encoding,
extlconv->mon_decimal_point);
- CurrentLocaleConv.mon_grouping = strdup(extlconv->mon_grouping);
- CurrentLocaleConv.mon_thousands_sep = db_encoding_strdup(encoding,
extlconv->mon_thousands_sep);
- CurrentLocaleConv.negative_sign = db_encoding_strdup(encoding,
extlconv->negative_sign);
- CurrentLocaleConv.positive_sign = db_encoding_strdup(encoding,
extlconv->positive_sign);
- CurrentLocaleConvAllocated = true;
+ tmplconv = *extlconv;
+ tmplconv.decimal_point = decimal_point;
+ tmplconv.grouping = grouping;
+ tmplconv.thousands_sep = thousands_sep;
+ tmplconv.int_curr_symbol = db_encoding_strdup(encoding,
extlconv->int_curr_symbol);
+ tmplconv.currency_symbol = db_encoding_strdup(encoding,
extlconv->currency_symbol);
+ tmplconv.mon_decimal_point = db_encoding_strdup(encoding,
extlconv->mon_decimal_point);
+ tmplconv.mon_grouping = strdup(extlconv->mon_grouping);
+ tmplconv.mon_thousands_sep = db_encoding_strdup(encoding,
extlconv->mon_thousands_sep);
+ tmplconv.negative_sign = db_encoding_strdup(encoding,
extlconv->negative_sign);
+ tmplconv.positive_sign = db_encoding_strdup(encoding,
extlconv->positive_sign);
/* Try to restore internal settings */
if (save_lc_monetary)
@@ -566,6 +566,31 @@ PGLC_localeconv(void)
}
#endif
+ /*
+ * Now that locales are set back to normal check for any NULL fields and
+ * report an out-of-memory error in consequence if any.
+ */
+ if (tmplconv.decimal_point == NULL ||
+ tmplconv.grouping == NULL ||
+ tmplconv.thousands_sep == NULL ||
+ tmplconv.int_curr_symbol == NULL ||
+ tmplconv.currency_symbol == NULL ||
+ tmplconv.mon_decimal_point == NULL ||
+ tmplconv.mon_grouping == NULL ||
+ tmplconv.mon_thousands_sep == NULL ||
+ tmplconv.negative_sign == NULL ||
+ tmplconv.positive_sign == NULL)
+ {
+ free_struct_lconv(&tmplconv);
+ elog(ERROR, "out of memory");
+ }
+
+ /*
+ * Everything is good, so save the result.
+ */
+ CurrentLocaleConv = tmplconv;
+
+ CurrentLocaleConvAllocated = true;
CurrentLocaleConvValid = true;
return &CurrentLocaleConv;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
