This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Tweak the handling of $HISTSIZE edge cases [PR gdb/16999]
- From: Pedro Alves <palves at redhat dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>, gdb-patches at sourceware dot org
- Date: Fri, 22 May 2015 00:33:52 +0100
- Subject: Re: [PATCH] Tweak the handling of $HISTSIZE edge cases [PR gdb/16999]
- Authentication-results: sourceware.org; auth=none
- References: <1432248648-7402-1-git-send-email-patrick at parcs dot ath dot cx>
On 05/21/2015 11:50 PM, Patrick Palka wrote:
The test is OK. Good use of with_test_prefix.
Minor nit: I found no other use of with_test_prefix with
spaces around the '='.
> diff --git a/gdb/top.c b/gdb/top.c
> index 74e1e07..38b4e5d 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -1684,17 +1684,21 @@ init_history (void)
> if (tmpenv)
> {
> int var;
> + char *endptr;
>
> - var = atoi (tmpenv);
> - if (var < 0)
> - {
> - /* Prefer ending up with no history rather than overflowing
> - readline's history interface, which uses signed 'int'
> - everywhere. */
> - var = 0;
> - }
> + var = strtol (tmpenv, &endptr, 10);
>
> - history_size_setshow_var = var;
> + /* If HISTSIZE is the empty string, negative, or non-numeric then set the
> + history size to unlimited. This behavior is mostly consistent with
> + that of bash. Whereas bash ignores a non-numeric HISTSIZE, we set the
> + history to unlimited in that case to avoid potentially truncating the
> + user's history. */
> + if (strlen (tmpenv) == 0
> + || var < 0
> + || *endptr != '\0')
If I'm reading correctly, this treats HISTSIZE=" " as "disable history".
Is that intended?
Also, a nit: I find it a bit odd to see strlen to check empty string
in one case, and != '\0' in another, instead of:
if (*tmpenv == '\0'
|| var < 0
|| *endptr != '\0')
> + history_size_setshow_var = -1;
> + else
> + history_size_setshow_var = var;
> }
> /* If the init file hasn't set a size yet, pick the default. */
> else if (history_size_setshow_var == -2)
Thanks,
Pedro Alves