From: dje@google.com
To: Yao Qi <yao@codesourcery.com>
Cc: <gdb-patches@sourceware.org>
Subject: Re: [PATCH 1/3] var_integer -> var_uinteger
Date: Thu, 23 Aug 2012 18:20:00 -0000 [thread overview]
Message-ID: <20534.29766.629459.204573@ruffy2.mtv.corp.google.com> (raw)
In-Reply-To: <1344871663-915-2-git-send-email-yao@codesourcery.com>
Yao Qi writes:
> Hi,
> The ngative value makes no sense to these commands, so this patch simply
> changes them to var_uinteger.
Hi. Most of the patch is ok with me.
Further comments inline.
> gdb:
>
> 2012-08-13 Yao Qi <yao@codesourcery.com>
>
> * cli/cli-cmds.c (max_user_call_depth): Add 'unsigned'.
> (init_cmds): Call add_setshow_uinteger_cmd for command
> 'max-user-call-depth'.
> * cli/cli-script.c (execute_user_command): Add 'unsigned' to the
> declaration of 'max_user_call_depth'.
> * frame.c (backtrace_limit): Add 'unsigned'.
> (_initialize_frame): Call add_setshow_uinteger_cmd for command
> 'limit'.
> * remote.c (remoteaddresssize): Add 'unsigned'.
> (remote_address_masked): Change local var 'address_size' to
> 'unsigned'.
> (_initialize_remote): Call add_setshow_uinteger_cmd for
> 'remoteaddresssize'.
> * top.c (history_size): Add 'unsigned'.
> (show_commands): Change local variables to 'unsigned'.
> (set_history_size_command): Don't check history_size is negative.
> [...]
> diff --git a/gdb/top.c b/gdb/top.c
> index 8251d1b..fbd4120 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -710,7 +710,7 @@ show_write_history_p (struct ui_file *file, int from_tty,
> value);
> }
>
> -static int history_size;
> +static unsigned int history_size;
> static void
> show_history_size (struct ui_file *file, int from_tty,
> struct cmd_list_element *c, const char *value)
> @@ -1379,7 +1379,7 @@ show_commands (char *args, int from_tty)
>
> /* The first command in the history which doesn't exist (i.e. one more
> than the number of the last command). Relative to history_base. */
> - int hist_len;
> + unsigned int hist_len;
>
> /* Print out some of the commands from the command history. */
> /* First determine the length of the history list. */
> @@ -1444,15 +1444,13 @@ show_commands (char *args, int from_tty)
> static void
> set_history_size_command (char *args, int from_tty, struct cmd_list_element *c)
> {
> - if (history_size == INT_MAX)
> + if (history_size == UINT_MAX)
> unstifle_history ();
> - else if (history_size >= 0)
> - stifle_history (history_size);
> + else if (history_size >= INT_MAX)
> + /* The type of parameter in stifle_history is int, so check the range. */
> + error (_("History size must be less than %d"), INT_MAX);
I think we don't want to remove resetting "history_size = INT_MAX;"
in the error case here (but see below).
Also, new style rules say that since the clause is multiple lines
it needs to be wrapped in {}.
It's kinda funny that explicitly setting the value to UINT_MAX (instead
of zero) is ok, but setting the value to anywhere between INT_MAX and
UINT_MAX-1 is an error because it's too big. :-)
That makes me think it'd be better to just bite the bullet and say
any value from INT_MAX and up means "unlimited".
They're effectively that anyway.
> else
> - {
> - history_size = INT_MAX;
> - error (_("History size must be non-negative"));
> - }
> + stifle_history (history_size);
> }
>
> void
> @@ -1632,13 +1630,13 @@ Without an argument, saving is enabled."),
> show_write_history_p,
> &sethistlist, &showhistlist);
>
> - add_setshow_integer_cmd ("size", no_class, &history_size, _("\
> + add_setshow_uinteger_cmd ("size", no_class, &history_size, _("\
> Set the size of the command history,"), _("\
> Show the size of the command history,"), _("\
> ie. the number of previous commands to keep a record of."),
> - set_history_size_command,
> - show_history_size,
> - &sethistlist, &showhistlist);
> + set_history_size_command,
> + show_history_size,
> + &sethistlist, &showhistlist);
>
> add_setshow_filename_cmd ("filename", no_class, &history_filename, _("\
> Set the filename in which to record the command history"), _("\
> --
> 1.7.7.6
>
next prev parent reply other threads:[~2012-08-23 18:20 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-18 12:52 [PATCH] Handle var_uinteger/var_zuinteger and case var_integer/var_zinteger together Yao Qi
2012-07-24 12:51 ` [committed]: " Yao Qi
2012-07-27 17:40 ` Khoo Yit Phang
2012-07-29 14:25 ` Yao Qi
2012-08-01 13:56 ` [RFC 0/3] New var_types var_zuinteger_unlimited Yao Qi
2012-08-01 13:56 ` [PATCH 1/3] var_zuinteger_unlimited and 'set listsize' Yao Qi
2012-08-01 16:14 ` Eli Zaretskii
2012-08-02 8:34 ` Doug Evans
2012-08-02 12:53 ` Yao Qi
2012-08-01 13:56 ` [PATCH 3/3] use zuinteger_unlimited for heuristic-fence-post Yao Qi
2012-08-01 13:56 ` [PATCH 2/3] use zuinteger_unlimited for some remote commands Yao Qi
2012-08-13 15:28 ` [RFC 0/3] Get rid of var_integer in CLI Yao Qi
2012-08-13 15:28 ` [PATCH 3/3] comments update Yao Qi
2012-09-14 18:13 ` Tom Tromey
2012-08-13 15:28 ` [PATCH 2/3] var_integer -> var_zuinteger_unlimited Yao Qi
2012-08-13 17:54 ` Eli Zaretskii
2012-09-14 18:12 ` Tom Tromey
2012-09-17 8:43 ` [committed]: " Yao Qi
2012-08-13 15:28 ` [PATCH 1/3] var_integer -> var_uinteger Yao Qi
2012-08-23 18:20 ` dje [this message]
2012-08-24 6:56 ` Yao Qi
2012-08-24 17:06 ` dje
2012-08-27 10:10 ` Yao Qi
2012-08-27 22:14 ` dje
2012-08-28 14:09 ` [committed]: " Yao Qi
2013-03-25 22:55 ` Change "set history size" back to signed (Re: [committed]: [PATCH 1/3] var_integer -> var_uinteger) Pedro Alves
2013-03-26 16:48 ` Yao Qi
2013-03-26 17:48 ` Pedro Alves
2013-03-27 8:54 ` Yao Qi
2013-03-27 17:34 ` Pedro Alves
2012-08-22 14:30 ` [ping] : [RFC 0/3] Get rid of var_integer in CLI Yao Qi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20534.29766.629459.204573@ruffy2.mtv.corp.google.com \
--to=dje@google.com \
--cc=gdb-patches@sourceware.org \
--cc=yao@codesourcery.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox