Re: [PATCH 1/3] var_integer -> var_uinteger

[dje at google dot 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
 >