[PATCH 1/2] Check the range

[PATCH 1/2] Check the range

[Yao Qi]

This patch is to check the range of various command input, and also
fixes one regression on 'set remote XXX-limit -1'.

  (gdb) set remote hardware-watchpoint-limit -1
  integer 4294967295 out of range

The new added tests expose existing regression on unpatched GDB,

FAIL: gdb.base/setshow.exp: set remote hardware-watchpoint-limit -1
FAIL: gdb.base/setshow.exp: show remote hardware-watchpoint-limit -1 (2)
FAIL: gdb.base/setshow.exp: set remote hardware-watchpoint-length-limit -1
FAIL: gdb.base/setshow.exp: show remote hardware-watchpoint-length-limit -1 (2)
FAIL: gdb.base/setshow.exp: set remote hardware-breakpoint-limit -1
FAIL: gdb.base/setshow.exp: show remote hardware-breakpoint-limit -1 (2)

This patch fixes these fails, but causes another regression,

  -PASS: gdb.base/list.exp: show listsize unlimited #7
  +FAIL: gdb.base/list.exp: show listsize unlimited #7

The next patch is to fix this regression on 'set listsize'.

gdb:

2012-09-09  Yao Qi  <yao@codesourcery.com>
	    Mark Kettenis  <kettenis@gnu.org>

	* cli/cli-setshow.c (do_set_command): Check the range of value
	for var_uinteger, var_zuinteger, var_zinteger and var_integer.

gdb/testsuite:

2012-09-09  Yao Qi  <yao@codesourcery.com>

	* gdb.base/setshow.exp: Test for setting ahd showing
	hardware-watchpoint-limit, hardware-watchpoint-length-limit and
	hardware-breakpoint-limit.
---
 gdb/cli/cli-setshow.c              |   12 ++++++++----
 gdb/testsuite/gdb.base/setshow.exp |   17 +++++++++++++++++
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/gdb/cli/cli-setshow.c b/gdb/cli/cli-setshow.c
index 9d8cb2e..7140479 100644
--- a/gdb/cli/cli-setshow.c
+++ b/gdb/cli/cli-setshow.c
@@ -275,7 +275,10 @@ do_set_command (char *arg, int from_tty, struct cmd_list_element *c)
       if (arg == NULL)
 	error_no_arg (_("integer to set it to."));
       {
-	unsigned int val = parse_and_eval_long (arg);
+	LONGEST val = parse_and_eval_long (arg);
+
+	if (val < 0 || val > UINT_MAX)
+	  error (_("integer %s out of range"), plongest (val));
 
 	if (c->var_type == var_uinteger && val == 0)
 	  val = UINT_MAX;
@@ -291,15 +294,16 @@ do_set_command (char *arg, int from_tty, struct cmd_list_element *c)
     case var_integer:
     case var_zinteger:
       {
-	unsigned int val;
+	LONGEST val;
 
 	if (arg == NULL)
 	  error_no_arg (_("integer to set it to."));
 	val = parse_and_eval_long (arg);
+
 	if (val == 0 && c->var_type == var_integer)
 	  val = INT_MAX;
-	else if (val >= INT_MAX)
-	  error (_("integer %u out of range"), val);
+	else if (val < INT_MIN || val >= INT_MAX)
+	  error (_("integer %s out of range"), plongest (val));
 
 	if (*(int *) c->var != val)
 	  {
diff --git a/gdb/testsuite/gdb.base/setshow.exp b/gdb/testsuite/gdb.base/setshow.exp
index d33e185..3a3e58f 100644
--- a/gdb/testsuite/gdb.base/setshow.exp
+++ b/gdb/testsuite/gdb.base/setshow.exp
@@ -259,3 +259,20 @@ gdb_test "show verbose" "Verbose printing of informational messages is on..*" "s
 gdb_test_no_output "set verbose off" "set verbose off" 
 #test show verbose off
 gdb_test "show verbose" "Verbosity is off..*" "show verbose (off)" 
+
+foreach remote_bpkt_option {
+    "hardware-watchpoint-limit"
+    "hardware-watchpoint-length-limit"
+    "hardware-breakpoint-limit"
+} {
+    gdb_test "show remote ${remote_bpkt_option}" \
+	"The maximum .* is -1.*" \
+	"show remote ${remote_bpkt_option} -1 (1)"
+    gdb_test_no_output "set remote ${remote_bpkt_option} 1"
+
+    gdb_test_no_output "set remote ${remote_bpkt_option} -1"
+    gdb_test "show remote ${remote_bpkt_option}" \
+	"The maximum .* is -1.*" \
+	"show remote ${remote_bpkt_option} -1 (2)"
+}
+
-- 
1.7.7.6

[2nd try, RFC 0/2] Fix 'set remote XXX-limit -1' and 'set listisize'

[Yao Qi]

Hi,
It was reported that my patch committed last month broke 'set remote
XXX-limit -1' by Khoo Yit Phang

  http://sourceware.org/ml/gdb-patches/2012-07/msg00702.html

and I proposed fix to this problem by adding a new type of 'var_type'
var_zuinteger_unlimited twice here,

  [RFC 0/3] New var_types var_zuinteger_unlimited
  http://sourceware.org/ml/gdb-patches/2012-08/msg00020.html

and

  [RFC 0/3] Get rid of var_integer in CLI
  http://sourceware.org/ml/gdb-patches/2012-08/msg00364.html

I feel that people are not convinced to add a new var_type
var_zuinteger_unlimited, so I decide to fix the problem in another way
(don't add new var_type).

Patch 1/2 adds the range check of command input, and patch 2/2 fixes
the inconsistency of the meaning of '0' and '-1' of 'listsize'.  I am
not sure it is a good idea to change the meaning of '0' and '-1' for
command 'set listsize' which has been used for many years, but it is
the best I can think of to fix the problem without adding new
var_type.  Comments are welcome.

[PATCH 2/2] Update doc and test of 'set listsize'.

[Yao Qi]

Hi,
This patch is to update the doc and test of 'set listsize'.  The
original version of doc was reviewed by Eli,

  http://sourceware.org/ml/gdb-patches/2012-08/msg00370.html

but I switch the meaning of '0' and '-1' of 'set listsize', because we
need two special states of 'listsize', 'unlimited' and 'suppress
printing'.  In existing 'enum var_types', only 0 means unlimited (in
var_uintger and var_integer') so it may be good to change 'set
listsize' to conform this, and let '-1' means suppress printing
accordingly.

gdb/doc:

2012-09-09  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (List): Describe the meaning of 0 and -1 in
	'set listsize'.

gdb/testsuite:

2012-09-09  Yao Qi  <yao@codesourcery.com>

	* gdb.base/list.exp (set_listsize): Set arg to "unlimited"
	when it is 0.
	(test_listsize): Adjust test for the different meaning
	of '0' and '-1' in listsize.  Remove 'setup_xfail'.
	Adjust the expected last line in of source in test.
---
 gdb/doc/gdb.texinfo             |    2 ++
 gdb/testsuite/gdb.base/list.exp |   13 ++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2d49e13..1bb2e5e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6706,6 +6706,8 @@ the @code{list} command.  You can change this using @code{set listsize}:
 @item set listsize @var{count}
 Make the @code{list} command display @var{count} source lines (unless
 the @code{list} argument explicitly specifies some other number).
+Setting @var{count} to 0 means there's no limit and -1 means suppress
+display of source lines.
 
 @kindex show listsize
 @item show listsize
diff --git a/gdb/testsuite/gdb.base/list.exp b/gdb/testsuite/gdb.base/list.exp
index 6b5b207..334db2a 100644
--- a/gdb/testsuite/gdb.base/list.exp
+++ b/gdb/testsuite/gdb.base/list.exp
@@ -62,7 +62,7 @@ proc set_listsize { arg } {
     if [gdb_test "set listsize $arg" ".*" "setting listsize to $arg #$set_listsize_count"] {
 	return 0;
     }
-    if { $arg <= 0 } {
+    if { $arg == 0 } {
 	set arg "unlimited";
     }
 
@@ -130,16 +130,15 @@ proc test_listsize {} {
 	gdb_test "list 10" "1\[ \t\]+#include \"list0.h\".*\r\n4\[23\]\[ \t\]+\}" "list line 10 with listsize 100"
     }
 
-    # Try listsize of 0 which suppresses printing.
+    # Try listsize of -1 which suppresses printing.
 
-    set_listsize 0
+    set_listsize -1
     gdb_test "list 1" "" "listsize of 0 suppresses output"
 
-    # Try listsize of -1 which is special, and means unlimited.
+    # Try listsize of 0 which is special, and means unlimited.
 
-    set_listsize -1
-    setup_xfail "*-*-*"
-    gdb_test "list 1" "1\[ \t\]+#include .*\r\n39\[ \t\]+\}" "list line 1 with unlimited listsize"
+    set_listsize 0
+    gdb_test "list 1" "1\[ \t\]+#include .*\r\n43\[ \t\]+\}" "list line 1 with unlimited listsize"
 }
 
 #
-- 
1.7.7.6

Re: [PATCH 2/2] Update doc and test of 'set listsize'.

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Sun, 9 Sep 2012 11:22:45 +0800
> 
> gdb/doc:
> 
> 2012-09-09  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.texinfo (List): Describe the meaning of 0 and -1 in
> 	'set listsize'.

OK for this part.

Thanks.

Re: [2nd try, RFC 0/2] Fix 'set remote XXX-limit -1' and 'set listisize'

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> I feel that people are not convinced to add a new var_type
Yao> var_zuinteger_unlimited, so I decide to fix the problem in another way
Yao> (don't add new var_type).

I thought it was fine; I think everybody is just very behind on patch
review.  But if you'd still rather go with this approach, let me know,
and I'll look.

Tom