On 2022-Jul-25, Andrew Dunstan wrote: > Committed. There were a couple of bits missing, which I filled in, and I > changed the behaviour when the option is given without an argument, so > that instead of resetting it the current value is shown, similarly to > how most pset options work.
I was translating the new messages introduced by this commit, and decided to unify them. While doing so, I notice that the feature misbehaves when you give it a string value that doesn't match any valid value: it just resets the value to 0, which is entirely the wrong thing. For other settings, we first verify that the given value is valid, and only then we change the setting. So I came up with the attached. While at it, I noticed that we have other uses of atoi() there, most notably pager_min_lines. My first impulse was to just to do the same as for xheader_width, namely to test whether the atoid result is zero, and not change in that case. But then I noticed we're already using variables.c facilities, so I decided to do likewise; this results in simpler code. So we're now stricter, because variables.c rejects trailing junk after the number. (123asd was previously parsed as 123, now it's an error.) There remain atoi uses in this code, and aside from the atoi()-induced behavior of making any non-number input set it to 0, it does stuff like =# \pset border 65538asd Border style is 2. because border style is short int, so this value overflows it. Same happens with 'columns'. However, this is all very old code and nobody seems to have complained. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ "¿Cómo puedes confiar en algo que pagas y que no ves, y no confiar en algo que te dan y te lo muestran?" (Germán Poo)
>From 5eb799984252cf231064d6d174e13d63880577df Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Fri, 19 May 2023 12:58:54 +0200 Subject: [PATCH] Tweak xheader_width input parsing Don't throw away the previous value when an invalid value is proposed. Also, change the error messages to not need separate translations. While at it, change pager_min_lines to also avoid changing the setting when an invalid value is given. --- src/bin/psql/command.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 607a57715a3..07c5f026b9b 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -4521,13 +4521,16 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) popt->topt.expanded_header_width_type = PRINT_XHEADER_PAGE; else { - popt->topt.expanded_header_width_type = PRINT_XHEADER_EXACT_WIDTH; - popt->topt.expanded_header_exact_width = atoi(value); - if (popt->topt.expanded_header_exact_width == 0) + int intval = atoi(value); + + if (intval == 0) { pg_log_error("\\pset: allowed xheader_width values are full (default), column, page, or a number specifying the exact width."); return false; } + + popt->topt.expanded_header_width_type = PRINT_XHEADER_EXACT_WIDTH; + popt->topt.expanded_header_exact_width = intval; } } @@ -4660,8 +4663,9 @@ do_pset(const char *param, const char *value, printQueryOpt *popt, bool quiet) /* set minimum lines for pager use */ else if (strcmp(param, "pager_min_lines") == 0) { - if (value) - popt->topt.pager_min_lines = atoi(value); + if (value && + !ParseVariableNum(value, "pager_min_lines", &popt->topt.pager_min_lines)) + return false; } /* disable "(x rows)" footer */ @@ -4727,11 +4731,11 @@ printPsetInfo(const char *param, printQueryOpt *popt) else if (strcmp(param, "xheader_width") == 0) { if (popt->topt.expanded_header_width_type == PRINT_XHEADER_FULL) - printf(_("Expanded header width is 'full'.\n")); + printf(_("Expanded header width is '%s'.\n"), "full"); else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_COLUMN) - printf(_("Expanded header width is 'column'.\n")); + printf(_("Expanded header width is '%s'.\n"), "column"); else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_PAGE) - printf(_("Expanded header width is 'page'.\n")); + printf(_("Expanded header width is '%s'.\n"), "page"); else if (popt->topt.expanded_header_width_type == PRINT_XHEADER_EXACT_WIDTH) printf(_("Expanded header width is %d.\n"), popt->topt.expanded_header_exact_width); } base-commit: 0b8ace8d773257fffeaceda196ed94877c2b74df -- 2.39.2