Andrew Dunstan <and...@dunslane.net> writes:
> On 05/13/2014 04:14 PM, Tom Lane wrote:
>> But independently of whether it's a fatal error or not: when there's
>> no relevant command-line argument then we print the
>>
>> invalid locale name ""
>>
>> message which is surely pretty unhelpful. It'd be better if we could
>> finger the incorrect environment setting. Unfortunately, we don't know
>> for sure which environment variable(s) setlocale was looking at --- I
>> believe it's somewhat platform specific. We could probably print
>> something like this instead:
>>
>> environment locale settings are invalid
>>
>> Thoughts?
> I'd also be tempted to add the settings for LC_ALL and LANG and note
> that they are possible sources of the problem, or maybe only do that if
> they match the locale being rejected.
Well, all we know is that we asked setlocale for the default locale from
the environment and it failed. We don't really know which variable(s)
it looked at. I think printing specific variables could easily be as
misleading as it is helpful; and given the lack of prior complaints,
I'm doubtful that it's worth going to the effort of trying to print a
platform-specific collection of variables.
Attached is a draft patch that behaves like this:
$ initdb --locale=foo
The files belonging to this database system will be owned by user "postgres".
This user must also own the server process.
initdb: invalid locale name "foo"
$ LANG=foo initdb
The files belonging to this database system will be owned by user "postgres".
This user must also own the server process.
initdb: environment locale settings are invalid
Most of the bulk of the patch is getting rid of the no-longer-useful
boolean result of check_locale_name.
regards, tom lane
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 2a51916..3a9dc53 100644
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
*************** static void trapsig(int signum);
*** 252,258 ****
static void check_ok(void);
static char *escape_quotes(const char *src);
static int locale_date_order(const char *locale);
! static bool check_locale_name(int category, const char *locale,
char **canonname);
static bool check_locale_encoding(const char *locale, int encoding);
static void setlocales(void);
--- 252,258 ----
static void check_ok(void);
static char *escape_quotes(const char *src);
static int locale_date_order(const char *locale);
! static void check_locale_name(int category, const char *locale,
char **canonname);
static bool check_locale_encoding(const char *locale, int encoding);
static void setlocales(void);
*************** locale_date_order(const char *locale)
*** 2529,2535 ****
}
/*
! * Is the locale name valid for the locale category?
*
* If successful, and canonname isn't NULL, a malloc'd copy of the locale's
* canonical name is stored there. This is especially useful for figuring out
--- 2529,2535 ----
}
/*
! * Verify that locale name is valid for the locale category.
*
* If successful, and canonname isn't NULL, a malloc'd copy of the locale's
* canonical name is stored there. This is especially useful for figuring out
*************** locale_date_order(const char *locale)
*** 2540,2546 ****
*
* this should match the backend's check_locale() function
*/
! static bool
check_locale_name(int category, const char *locale, char **canonname)
{
char *save;
--- 2540,2546 ----
*
* this should match the backend's check_locale() function
*/
! static void
check_locale_name(int category, const char *locale, char **canonname)
{
char *save;
*************** check_locale_name(int category, const ch
*** 2551,2557 ****
save = setlocale(category, NULL);
if (!save)
! return false; /* won't happen, we hope */
/* save may be pointing at a modifiable scratch variable, so copy it. */
save = pg_strdup(save);
--- 2551,2561 ----
save = setlocale(category, NULL);
if (!save)
! {
! fprintf(stderr, _("%s: setlocale failed\n"),
! progname);
! exit(1);
! }
/* save may be pointing at a modifiable scratch variable, so copy it. */
save = pg_strdup(save);
*************** check_locale_name(int category, const ch
*** 2565,2580 ****
/* restore old value. */
if (!setlocale(category, save))
fprintf(stderr, _("%s: failed to restore old locale \"%s\"\n"),
progname, save);
free(save);
! /* should we exit here? */
if (res == NULL)
! fprintf(stderr, _("%s: invalid locale name \"%s\"\n"),
! progname, locale);
!
! return (res != NULL);
}
/*
--- 2569,2602 ----
/* restore old value. */
if (!setlocale(category, save))
+ {
fprintf(stderr, _("%s: failed to restore old locale \"%s\"\n"),
progname, save);
+ exit(1);
+ }
free(save);
! /* complain if locale wasn't valid */
if (res == NULL)
! {
! if (*locale)
! fprintf(stderr, _("%s: invalid locale name \"%s\"\n"),
! progname, locale);
! else
! {
! /*
! * If no relevant switch was given on command line, locale is an
! * empty string, which is not too helpful to report. Presumably
! * setlocale() found something it did not like in the environment.
! * Ideally we'd report the bad environment variable, but since
! * setlocale's behavior is implementation-specific, it's hard to
! * be sure what it didn't like. Print a safe generic message.
! */
! fprintf(stderr, _("%s: environment locale settings are invalid\n"),
! progname);
! }
! exit(1);
! }
}
/*
*************** setlocales(void)
*** 2642,2682 ****
}
/*
! * canonicalize locale names, and override any missing/invalid values from
! * our current environment
*/
! if (check_locale_name(LC_CTYPE, lc_ctype, &canonname))
! lc_ctype = canonname;
! else
! lc_ctype = pg_strdup(setlocale(LC_CTYPE, NULL));
! if (check_locale_name(LC_COLLATE, lc_collate, &canonname))
! lc_collate = canonname;
! else
! lc_collate = pg_strdup(setlocale(LC_COLLATE, NULL));
! if (check_locale_name(LC_NUMERIC, lc_numeric, &canonname))
! lc_numeric = canonname;
! else
! lc_numeric = pg_strdup(setlocale(LC_NUMERIC, NULL));
! if (check_locale_name(LC_TIME, lc_time, &canonname))
! lc_time = canonname;
! else
! lc_time = pg_strdup(setlocale(LC_TIME, NULL));
! if (check_locale_name(LC_MONETARY, lc_monetary, &canonname))
! lc_monetary = canonname;
! else
! lc_monetary = pg_strdup(setlocale(LC_MONETARY, NULL));
#if defined(LC_MESSAGES) && !defined(WIN32)
! if (check_locale_name(LC_MESSAGES, lc_messages, &canonname))
! lc_messages = canonname;
! else
! lc_messages = pg_strdup(setlocale(LC_MESSAGES, NULL));
#else
/* when LC_MESSAGES is not available, use the LC_CTYPE setting */
! if (check_locale_name(LC_CTYPE, lc_messages, &canonname))
! lc_messages = canonname;
! else
! lc_messages = pg_strdup(setlocale(LC_CTYPE, NULL));
#endif
}
--- 2664,2690 ----
}
/*
! * canonicalize locale names, and obtain any missing values from our
! * current environment
*/
! check_locale_name(LC_CTYPE, lc_ctype, &canonname);
! lc_ctype = canonname;
! check_locale_name(LC_COLLATE, lc_collate, &canonname);
! lc_collate = canonname;
! check_locale_name(LC_NUMERIC, lc_numeric, &canonname);
! lc_numeric = canonname;
! check_locale_name(LC_TIME, lc_time, &canonname);
! lc_time = canonname;
! check_locale_name(LC_MONETARY, lc_monetary, &canonname);
! lc_monetary = canonname;
#if defined(LC_MESSAGES) && !defined(WIN32)
! check_locale_name(LC_MESSAGES, lc_messages, &canonname);
! lc_messages = canonname;
#else
/* when LC_MESSAGES is not available, use the LC_CTYPE setting */
! check_locale_name(LC_CTYPE, lc_messages, &canonname);
! lc_messages = canonname;
#endif
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
