[RFA] add test for memattr, use get_number_or_range for memattr commands

[RFA] add test for memattr, use get_number_or_range for memattr commands

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 103 bytes --]

Before modifying these commands, I noticed there was no test for them,
so I wrote one.

OK to commit?


[-- Attachment #2: memattr.txt --]
[-- Type: text/plain, Size: 15010 bytes --]

2011-02-19  Michael Snyder  <msnyder@vmware.com>

	* memattr.c (mem_enable_command): Use get_number_or_range.
	(mem_disable_command): Ditto.
	(mem_delete_command): Ditto.

2011-02-19  Michael Snyder  <msnyder@vmware.com>

	* gdb.base/memattr.exp: New test.
	* gdb.base/memattr.c: Test load for memattr.exp.

Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.42
diff -u -p -u -p -r1.42 memattr.c
--- memattr.c	9 Jan 2011 03:20:33 -0000	1.42
+++ memattr.c	19 Feb 2011 23:25:11 -0000
@@ -27,6 +27,7 @@
 #include "language.h"
 #include "vec.h"
 #include "gdb_string.h"
+#include "breakpoint.h"
 
 const struct mem_attrib default_mem_attrib =
 {
@@ -562,8 +563,6 @@ mem_enable (int num)
 static void
 mem_enable_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
   struct mem_region *m;
   int ix;
@@ -572,26 +571,16 @@ mem_enable_command (char *args, int from
 
   target_dcache_invalidate ();
 
-  if (p == 0)
-    {
+  if (args == NULL || *args == '\0')
+    { /* Enable all mem regions.  */
       for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
 	m->enabled_p = 1;
     }
   else
-    while (*p)
+    while (args != NULL && *args != '\0')
       {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be memory region numbers."));
-
-	num = atoi (p);
+	num = get_number_or_range (&args);
 	mem_enable (num);
-
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
       }
 }
 \f
@@ -616,8 +605,6 @@ mem_disable (int num)
 static void
 mem_disable_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
   struct mem_region *m;
   int ix;
@@ -626,26 +613,16 @@ mem_disable_command (char *args, int fro
 
   target_dcache_invalidate ();
 
-  if (p == 0)
+  if (args == NULL || *args == '\0')
     {
       for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
 	m->enabled_p = 0;
     }
   else
-    while (*p)
+    while (args != NULL && *args != '\0')
       {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be memory region numbers."));
-
-	num = atoi (p);
+	num = get_number_or_range (&args);
 	mem_disable (num);
-
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
       }
 }
 
@@ -679,15 +656,13 @@ mem_delete (int num)
 static void
 mem_delete_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
 
   require_user_regions (from_tty);
 
   target_dcache_invalidate ();
 
-  if (p == 0)
+  if (args == NULL || *args == '\0')
     {
       if (query (_("Delete all memory regions? ")))
 	mem_clear ();
@@ -695,20 +670,10 @@ mem_delete_command (char *args, int from
       return;
     }
 
-  while (*p)
+  while (args != NULL && *args != '\0')
     {
-      p1 = p;
-      while (*p1 >= '0' && *p1 <= '9')
-	p1++;
-      if (*p1 && *p1 != ' ' && *p1 != '\t')
-	error (_("Arguments must be memory region numbers."));
-
-      num = atoi (p);
+      num = get_number_or_range (&args);
       mem_delete (num);
-
-      p = p1;
-      while (*p == ' ' || *p == '\t')
-	p++;
     }
 
   dont_repeat ();
@@ -739,19 +704,19 @@ where <mode>  may be rw (read/write), ro
   add_cmd ("mem", class_vars, mem_enable_command, _("\
 Enable memory region.\n\
 Arguments are the code numbers of the memory regions to enable.\n\
-Usage: enable mem <code number>\n\
+Usage: enable mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &enablelist);
 
   add_cmd ("mem", class_vars, mem_disable_command, _("\
 Disable memory region.\n\
 Arguments are the code numbers of the memory regions to disable.\n\
-Usage: disable mem <code number>\n\
+Usage: disable mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &disablelist);
 
   add_cmd ("mem", class_vars, mem_delete_command, _("\
 Delete memory region.\n\
 Arguments are the code numbers of the memory regions to delete.\n\
-Usage: delete mem <code number>\n\
+Usage: delete mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &deletelist);
 
   add_info ("mem", mem_info_command,
Index: testsuite/gdb.base/memattr.c
===================================================================
RCS file: testsuite/gdb.base/memattr.c
diff -N testsuite/gdb.base/memattr.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/memattr.c	19 Feb 2011 23:25:11 -0000
@@ -0,0 +1,11 @@
+#define MEMSIZE 64
+static int mem1[MEMSIZE] = {111, 222, 333, 444, 555};
+static int mem2[MEMSIZE];
+static int mem3[MEMSIZE];
+static int mem4[MEMSIZE];
+static int mem5[MEMSIZE];
+
+int main()
+{
+  return 0;
+}
Index: testsuite/gdb.base/memattr.exp
===================================================================
RCS file: testsuite/gdb.base/memattr.exp
diff -N testsuite/gdb.base/memattr.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/memattr.exp	19 Feb 2011 23:25:11 -0000
@@ -0,0 +1,419 @@
+# Copyright 1998, 1999, 2000, 2007, 2008, 2009, 2010, 2011
+# Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite
+
+# Test the memory attribute commands.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile "memattr"
+set srcfile  ${testfile}.c
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    return -1
+}
+
+runto main
+
+gdb_test_multiple "info address mem1" "get address of mem1" {
+    -re "Symbol \"mem1\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem1start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem2" "get address of mem2" {
+    -re "Symbol \"mem2\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem2start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem3" "get address of mem3" {
+    -re "Symbol \"mem3\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem3start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem4" "get address of mem4" {
+    -re "Symbol \"mem4\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem4start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem5" "get address of mem5" {
+    -re "Symbol \"mem5\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem5start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem1\[64\]" "get end of mem1" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem1end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem2\[64\]" "get end of mem2" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem2end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem3\[64\]" "get end of mem3" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem3end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem4\[64\]" "get end of mem4" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem4end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem5\[64\]" "get end of mem5" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem5end $expect_out(1,string)
+    }
+}
+
+gdb_test_no_output "mem $mem1start $mem1end wo" "mem 1"
+gdb_test_no_output "mem $mem2start $mem2end ro" "mem 2"
+gdb_test_no_output "mem $mem3start $mem3end rw" "mem 3"
+gdb_test_no_output "mem $mem4start $mem4end rw" "mem 4"
+gdb_test_no_output "mem $mem5start $mem5end rw" "mem 5"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "info mem(1)" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "info mem (1)"
+	} else {
+	    fail "info mem (1)"
+	}
+    }
+}
+
+#
+# Test disable and enable
+#
+
+gdb_test_no_output "disable mem 1" "disable mem 1"
+gdb_test "info mem" "1   n  .*" "mem 1 was disabled"
+
+gdb_test_no_output "enable mem 1" "enable mem 1"
+gdb_test "info mem" "1   y  .*" "mem 1 was enabled"
+
+gdb_test_no_output "disable mem 2 4"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 2 and 4 were disabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   n  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 2 and 4 were disabled"
+	} else {
+	    fail "mem 2 and 4 were disabled"
+	}
+    }
+}
+
+gdb_test_no_output "enable mem 2-4" "enable mem 2-4"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 2-4 were enabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 2-4 were enabled"
+	} else {
+	    fail "mem 2-4 were enabled"
+	}
+    }
+}
+
+gdb_test_no_output "disable mem" "disable mem"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 1 to 5 were disabled" {
+    -re "1   n  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   n  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   n  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 to 5 were disabled"
+	} else {
+	    fail "mem 1 to 5 were disabled"
+	}
+    }
+}
+
+gdb_test_no_output "enable mem" "enable mem"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 1 to 5 were enabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 to 5 were enabled"
+	} else {
+	    fail "mem 1 to 5 were enabled"
+	}
+    }
+}
+
+gdb_test "disable mem 7 8" \
+    "No memory region number 7.*No memory region number 8." \
+    "disable mem 7 8"
+
+#
+# Test delete
+#
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_no_output "delete mem 1" "delete mem 1"
+gdb_test_multiple "info mem" "mem 1 was deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 was deleted"
+	} else {
+	    fail "mem 1 was deleted"
+	}
+    }
+}
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_no_output "delete mem 2 4" "delete mem 2 4"
+gdb_test_multiple "info mem" "mem 2 and 4 were deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && !$see2 && $see3 && !$see4 && $see5 } then {
+	    pass "mem 2 and 4 were deleted"
+	} else {
+	    fail "mem 2 and 4 were deleted"
+	}
+    }
+}
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test "delete mem 2-4" \
+    "No memory region number 2.*No memory region number 4." \
+    "delete mem 2-4"
+gdb_test_multiple "info mem" "mem 2-4 were deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && !$see2 && !$see3 && !$see4 && $see5 } then {
+	    pass "mem 2-4 were deleted"
+	} else {
+	    fail "mem 2-4 were deleted"
+	}
+    }
+}
+
+gdb_test "delete mem 8" "No memory region number 8." "delete mem 8"

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Joel Brobecker]

> 2011-02-19  Michael Snyder  <msnyder@vmware.com>
> 
> 	* memattr.c (mem_enable_command): Use get_number_or_range.
> 	(mem_disable_command): Ditto.
> 	(mem_delete_command): Ditto.

You forgot in the ChangeLog the documentation updates (which should
be reviewed by Eli).

The code changes look fine to me.

> 2011-02-19  Michael Snyder  <msnyder@vmware.com>
> 
> 	* gdb.base/memattr.exp: New test.
> 	* gdb.base/memattr.c: Test load for memattr.exp.

> Index: testsuite/gdb.base/memattr.c
> ===================================================================
> RCS file: testsuite/gdb.base/memattr.c
> diff -N testsuite/gdb.base/memattr.c
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.base/memattr.c	19 Feb 2011 23:25:11 -0000
> @@ -0,0 +1,11 @@
> +#define MEMSIZE 64

This file needs a copyright header (with the copyright year starting
with 2011 - see below :-).


> Index: testsuite/gdb.base/memattr.exp
> ===================================================================
> RCS file: testsuite/gdb.base/memattr.exp
> diff -N testsuite/gdb.base/memattr.exp
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ testsuite/gdb.base/memattr.exp	19 Feb 2011 23:25:11 -0000
> @@ -0,0 +1,419 @@
> +# Copyright 1998, 1999, 2000, 2007, 2008, 2009, 2010, 2011
> +# Free Software Foundation, Inc.

This is one of these things were I'm not totally sure.  But should
we really claim copyright on this file all the way up to 2011?
ISTM that we can only claim starting in 2011, since this file
was presumably created in 2011.  I don't think that the fact that
you started from another file and duplicated a small part of it
is sufficient to claim that it existed since 1998...

> +gdb_test_multiple "info address mem1" "get address of mem1" {
> +    -re "Symbol \"mem1\" is static storage at address ($hex).*$gdb_prompt $" {
> +	set mem1start $expect_out(1,string)
> +    }
> +}

What happens if this operation fails? I think that the testcase will
badly crash as soon as you start using $mem1start, no? Should we fail
& return? Or just perform all the address extraction first, and then
have a test that verifies the existence of every variable before
continuing with the rest of the test?  I find the latter suggestion
easier to implement, but a little more dangerous.

-- 
Joel

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This is one of these things were I'm not totally sure.  But should
Joel> we really claim copyright on this file all the way up to 2011?
Joel> ISTM that we can only claim starting in 2011, since this file
Joel> was presumably created in 2011.  I don't think that the fact that
Joel> you started from another file and duplicated a small part of it
Joel> is sufficient to claim that it existed since 1998...

I leave all the dates if I think that the copied part is significant.  I
am not certain that this is the right thing to do though :-)

Tom

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Michael Snyder]

Tom Tromey wrote:
>>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> 
> Joel> This is one of these things were I'm not totally sure.  But should
> Joel> we really claim copyright on this file all the way up to 2011?
> Joel> ISTM that we can only claim starting in 2011, since this file
> Joel> was presumably created in 2011.  I don't think that the fact that
> Joel> you started from another file and duplicated a small part of it
> Joel> is sufficient to claim that it existed since 1998...
> 
> I leave all the dates if I think that the copied part is significant.  I
> am not certain that this is the right thing to do though :-)

It was a cut and paste accident, I will remove them.

Michael

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

Tom Tromey wrote:
>>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
> 
> Joel> This is one of these things were I'm not totally sure.  But should
> Joel> we really claim copyright on this file all the way up to 2011?
> Joel> ISTM that we can only claim starting in 2011, since this file
> Joel> was presumably created in 2011.  I don't think that the fact that
> Joel> you started from another file and duplicated a small part of it
> Joel> is sufficient to claim that it existed since 1998...
> 
> I leave all the dates if I think that the copied part is significant.  I
> am not certain that this is the right thing to do though :-)


OK, updated patch with new boilerplate, copyright dates and changelog 
corrected.


[-- Attachment #2: memattr2.txt --]
[-- Type: text/plain, Size: 16401 bytes --]

2011-02-21  Michael Snyder  <msnyder@vmware.com>

	* memattr.c (mem_enable_command): Use get_number_or_range.
	(mem_disable_command): Ditto.
	(mem_delete_command): Ditto.
	(_initialize_mem): Tweak usage message to reflect multiple
	arguments.

2011-02-21  Michael Snyder  <msnyder@vmware.com>

	* gdb.base/memattr.exp: New test.
	* gdb.base/memattr.c: Test load for memattr.exp.

Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.42
diff -u -p -u -p -r1.42 memattr.c
--- memattr.c	9 Jan 2011 03:20:33 -0000	1.42
+++ memattr.c	21 Feb 2011 22:54:05 -0000
@@ -27,6 +27,7 @@
 #include "language.h"
 #include "vec.h"
 #include "gdb_string.h"
+#include "breakpoint.h"
 
 const struct mem_attrib default_mem_attrib =
 {
@@ -562,8 +563,6 @@ mem_enable (int num)
 static void
 mem_enable_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
   struct mem_region *m;
   int ix;
@@ -572,26 +571,16 @@ mem_enable_command (char *args, int from
 
   target_dcache_invalidate ();
 
-  if (p == 0)
-    {
+  if (args == NULL || *args == '\0')
+    { /* Enable all mem regions.  */
       for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
 	m->enabled_p = 1;
     }
   else
-    while (*p)
+    while (args != NULL && *args != '\0')
       {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be memory region numbers."));
-
-	num = atoi (p);
+	num = get_number_or_range (&args);
 	mem_enable (num);
-
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
       }
 }
 \f
@@ -616,8 +605,6 @@ mem_disable (int num)
 static void
 mem_disable_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
   struct mem_region *m;
   int ix;
@@ -626,26 +613,16 @@ mem_disable_command (char *args, int fro
 
   target_dcache_invalidate ();
 
-  if (p == 0)
+  if (args == NULL || *args == '\0')
     {
       for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
 	m->enabled_p = 0;
     }
   else
-    while (*p)
+    while (args != NULL && *args != '\0')
       {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be memory region numbers."));
-
-	num = atoi (p);
+	num = get_number_or_range (&args);
 	mem_disable (num);
-
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
       }
 }
 
@@ -679,15 +656,13 @@ mem_delete (int num)
 static void
 mem_delete_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
 
   require_user_regions (from_tty);
 
   target_dcache_invalidate ();
 
-  if (p == 0)
+  if (args == NULL || *args == '\0')
     {
       if (query (_("Delete all memory regions? ")))
 	mem_clear ();
@@ -695,20 +670,10 @@ mem_delete_command (char *args, int from
       return;
     }
 
-  while (*p)
+  while (args != NULL && *args != '\0')
     {
-      p1 = p;
-      while (*p1 >= '0' && *p1 <= '9')
-	p1++;
-      if (*p1 && *p1 != ' ' && *p1 != '\t')
-	error (_("Arguments must be memory region numbers."));
-
-      num = atoi (p);
+      num = get_number_or_range (&args);
       mem_delete (num);
-
-      p = p1;
-      while (*p == ' ' || *p == '\t')
-	p++;
     }
 
   dont_repeat ();
@@ -739,19 +704,19 @@ where <mode>  may be rw (read/write), ro
   add_cmd ("mem", class_vars, mem_enable_command, _("\
 Enable memory region.\n\
 Arguments are the code numbers of the memory regions to enable.\n\
-Usage: enable mem <code number>\n\
+Usage: enable mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &enablelist);
 
   add_cmd ("mem", class_vars, mem_disable_command, _("\
 Disable memory region.\n\
 Arguments are the code numbers of the memory regions to disable.\n\
-Usage: disable mem <code number>\n\
+Usage: disable mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &disablelist);
 
   add_cmd ("mem", class_vars, mem_delete_command, _("\
 Delete memory region.\n\
 Arguments are the code numbers of the memory regions to delete.\n\
-Usage: delete mem <code number>\n\
+Usage: delete mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &deletelist);
 
   add_info ("mem", mem_info_command,
 
Index: testsuite/gdb.base/memattr.c
===================================================================
RCS file: testsuite/gdb.base/memattr.c
diff -N testsuite/gdb.base/memattr.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/memattr.c	21 Feb 2011 22:56:41 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define MEMSIZE 64
+static int mem1[MEMSIZE] = {111, 222, 333, 444, 555};
+static int mem2[MEMSIZE];
+static int mem3[MEMSIZE];
+static int mem4[MEMSIZE];
+static int mem5[MEMSIZE];
+
+int main()
+{
+  return 0;
+}
Index: testsuite/gdb.base/memattr.exp
===================================================================
RCS file: testsuite/gdb.base/memattr.exp
diff -N testsuite/gdb.base/memattr.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/memattr.exp	21 Feb 2011 22:56:41 -0000
@@ -0,0 +1,442 @@
+# Copyright 2011
+# Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite
+
+# Test the memory attribute commands.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile "memattr"
+set srcfile  ${testfile}.c
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    return -1
+}
+
+runto main
+
+gdb_test_multiple "info address mem1" "get address of mem1" {
+    -re "Symbol \"mem1\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem1start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem2" "get address of mem2" {
+    -re "Symbol \"mem2\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem2start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem3" "get address of mem3" {
+    -re "Symbol \"mem3\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem3start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem4" "get address of mem4" {
+    -re "Symbol \"mem4\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem4start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem5" "get address of mem5" {
+    -re "Symbol \"mem5\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem5start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem1\[64\]" "get end of mem1" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem1end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem2\[64\]" "get end of mem2" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem2end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem3\[64\]" "get end of mem3" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem3end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem4\[64\]" "get end of mem4" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem4end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem5\[64\]" "get end of mem5" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem5end $expect_out(1,string)
+    }
+}
+
+gdb_test_no_output "mem $mem1start $mem1end wo" "create mem region 1"
+gdb_test_no_output "mem $mem2start $mem2end ro" "create mem region 2"
+gdb_test_no_output "mem $mem3start $mem3end rw" "create mem region 3"
+gdb_test_no_output "mem $mem4start $mem4end rw" "create mem region 4"
+gdb_test_no_output "mem $mem5start $mem5end rw" "create mem region 5"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "info mem(1)" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "info mem (1)"
+	} else {
+	    fail "info mem (1)"
+	}
+    }
+}
+
+#
+# Test read-only, write-only
+#
+
+# mem1 is write only: read should fail.
+gdb_test "print mem1\[1\]" \
+    "Cannot access memory at address $hex" \
+    "mem1 cannot be read"
+
+gdb_test "print mem1\[1\] = 9" \
+    "$decimal = 9" \
+    "mem1 can be written"
+
+# mem2 is read only: write should fail.
+gdb_test "print mem2\[1\] = 9" \
+    "Cannot access memory at address $hex" \
+    "mem2 cannot be written"
+
+gdb_test "print mem2\[1\]" \
+    "$decimal = 0" \
+    "mem2 can be read"
+
+#
+# Test disable and enable
+#
+
+gdb_test_no_output "disable mem 1" "disable mem 1"
+gdb_test "info mem" "1   n  .*" "mem 1 was disabled"
+
+gdb_test_no_output "enable mem 1" "enable mem 1"
+gdb_test "info mem" "1   y  .*" "mem 1 was enabled"
+
+gdb_test_no_output "disable mem 2 4"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 2 and 4 were disabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   n  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 2 and 4 were disabled"
+	} else {
+	    fail "mem 2 and 4 were disabled"
+	}
+    }
+}
+
+gdb_test_no_output "enable mem 2-4" "enable mem 2-4"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 2-4 were enabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 2-4 were enabled"
+	} else {
+	    fail "mem 2-4 were enabled"
+	}
+    }
+}
+
+gdb_test_no_output "disable mem" "disable mem"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 1 to 5 were disabled" {
+    -re "1   n  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   n  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   n  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 to 5 were disabled"
+	} else {
+	    fail "mem 1 to 5 were disabled"
+	}
+    }
+}
+
+gdb_test_no_output "enable mem" "enable mem"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 1 to 5 were enabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 to 5 were enabled"
+	} else {
+	    fail "mem 1 to 5 were enabled"
+	}
+    }
+}
+
+gdb_test "disable mem 7 8" \
+    "No memory region number 7.*No memory region number 8." \
+    "disable non-existant regions"
+
+#
+# Test delete
+#
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_no_output "delete mem 1" "delete mem 1"
+gdb_test_multiple "info mem" "mem 1 was deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 was deleted"
+	} else {
+	    fail "mem 1 was deleted"
+	}
+    }
+}
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_no_output "delete mem 2 4" "delete mem 2 4"
+gdb_test_multiple "info mem" "mem 2 and 4 were deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && !$see2 && $see3 && !$see4 && $see5 } then {
+	    pass "mem 2 and 4 were deleted"
+	} else {
+	    fail "mem 2 and 4 were deleted"
+	}
+    }
+}
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test "delete mem 2-4" \
+    "No memory region number 2.*No memory region number 4." \
+    "delete mem 2-4"
+gdb_test_multiple "info mem" "mem 2-4 were deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && !$see2 && !$see3 && !$see4 && $see5 } then {
+	    pass "mem 2-4 were deleted"
+	} else {
+	    fail "mem 2-4 were deleted"
+	}
+    }
+}
+
+gdb_test "delete mem 8" "No memory region number 8." \
+    "delete non-existant region"

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Joel Brobecker]

> 2011-02-21  Michael Snyder  <msnyder@vmware.com>
> 
> 	* memattr.c (mem_enable_command): Use get_number_or_range.
> 	(mem_disable_command): Ditto.
> 	(mem_delete_command): Ditto.
> 	(_initialize_mem): Tweak usage message to reflect multiple
> 	arguments.
> 
> 2011-02-21  Michael Snyder  <msnyder@vmware.com>
> 
> 	* gdb.base/memattr.exp: New test.
> 	* gdb.base/memattr.c: Test load for memattr.exp.

Overall, the patch looks OK to me.

> +gdb_test_multiple "info address mem1" "get address of mem1" {
> +    -re "Symbol \"mem1\" is static storage at address ($hex).*$gdb_prompt $" {
> +	set mem1start $expect_out(1,string)
> +    }
> +}
[...]
> +gdb_test_no_output "mem $mem1start $mem1end wo" "create mem region 1"

I still think that, if the first quoted test does not pass, the second
will cause the testcase to crash because mem1start is going to be
undefined. This is what you're going to see when that happens:

    ERROR: tcl error sourcing /[...]/memattr.exp
    ERROR: can't read "mem1start": no such variable
        while executing
    "gdb_test_no_output "mem $mem1start $mem1end wo" "create mem region 1""

But I'm OK with that, if that's OK with the others.

-- 
Joel

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Michael Snyder]

Joel Brobecker wrote:
>> 2011-02-21  Michael Snyder  <msnyder@vmware.com>
>>
>> 	* memattr.c (mem_enable_command): Use get_number_or_range.
>> 	(mem_disable_command): Ditto.
>> 	(mem_delete_command): Ditto.
>> 	(_initialize_mem): Tweak usage message to reflect multiple
>> 	arguments.
>>
>> 2011-02-21  Michael Snyder  <msnyder@vmware.com>
>>
>> 	* gdb.base/memattr.exp: New test.
>> 	* gdb.base/memattr.c: Test load for memattr.exp.
> 
> Overall, the patch looks OK to me.
> 
>> +gdb_test_multiple "info address mem1" "get address of mem1" {
>> +    -re "Symbol \"mem1\" is static storage at address ($hex).*$gdb_prompt $" {
>> +	set mem1start $expect_out(1,string)
>> +    }
>> +}
> [...]
>> +gdb_test_no_output "mem $mem1start $mem1end wo" "create mem region 1"
> 
> I still think that, if the first quoted test does not pass, the second
> will cause the testcase to crash because mem1start is going to be
> undefined. This is what you're going to see when that happens:
> 
>     ERROR: tcl error sourcing /[...]/memattr.exp
>     ERROR: can't read "mem1start": no such variable
>         while executing
>     "gdb_test_no_output "mem $mem1start $mem1end wo" "create mem region 1""
> 
> But I'm OK with that, if that's OK with the others.

Hmmm, how about if I initialize mem1start etc. to -1?
Then the rest of the tests will fail but not crash.
Is that better?

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Joel Brobecker]

> Hmmm, how about if I initialize mem1start etc. to -1?
> Then the rest of the tests will fail but not crash.
> Is that better?

Yep, that would work for me. You could also do something like this:

    if {![info exists mem1start] || ![info exists mem2start] ...} {
        fail "..."
        # No point in continuing the rest of the testcase if we couldn't
        # get the addresses we need bla bla bla.
        return
    }

-- 
Joel

Re: [RFA] add test for memattr, use get_number_or_range for memattr commands

[Michael Snyder]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Joel Brobecker wrote:
>> Hmmm, how about if I initialize mem1start etc. to -1?
>> Then the rest of the tests will fail but not crash.
>> Is that better?
> 
> Yep, that would work for me. You could also do something like this:
> 
>     if {![info exists mem1start] || ![info exists mem2start] ...} {
>         fail "..."
>         # No point in continuing the rest of the testcase if we couldn't
>         # get the addresses we need bla bla bla.
>         return
>     }
> 

OK, committed as below.



[-- Attachment #2: memattr3.txt --]
[-- Type: text/plain, Size: 16575 bytes --]

2011-02-22  Michael Snyder  <msnyder@vmware.com>

	* memattr.c (mem_enable_command): Use get_number_or_range.
	(mem_disable_command): Ditto.
	(mem_delete_command): Ditto.
	(_initialize_mem): Tweak usage message to reflect multiple
	arguments.

2011-02-22  Michael Snyder  <msnyder@vmware.com>

	* gdb.base/memattr.exp: New test.
	* gdb.base/memattr.c: Test load for memattr.exp.

Index: memattr.c
===================================================================
RCS file: /cvs/src/src/gdb/memattr.c,v
retrieving revision 1.42
diff -u -p -u -p -r1.42 memattr.c
--- memattr.c	9 Jan 2011 03:20:33 -0000	1.42
+++ memattr.c	23 Feb 2011 18:28:43 -0000
@@ -27,6 +27,7 @@
 #include "language.h"
 #include "vec.h"
 #include "gdb_string.h"
+#include "breakpoint.h"
 
 const struct mem_attrib default_mem_attrib =
 {
@@ -562,8 +563,6 @@ mem_enable (int num)
 static void
 mem_enable_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
   struct mem_region *m;
   int ix;
@@ -572,26 +571,16 @@ mem_enable_command (char *args, int from
 
   target_dcache_invalidate ();
 
-  if (p == 0)
-    {
+  if (args == NULL || *args == '\0')
+    { /* Enable all mem regions.  */
       for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
 	m->enabled_p = 1;
     }
   else
-    while (*p)
+    while (args != NULL && *args != '\0')
       {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be memory region numbers."));
-
-	num = atoi (p);
+	num = get_number_or_range (&args);
 	mem_enable (num);
-
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
       }
 }
 \f
@@ -616,8 +605,6 @@ mem_disable (int num)
 static void
 mem_disable_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
   struct mem_region *m;
   int ix;
@@ -626,26 +613,16 @@ mem_disable_command (char *args, int fro
 
   target_dcache_invalidate ();
 
-  if (p == 0)
+  if (args == NULL || *args == '\0')
     {
       for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
 	m->enabled_p = 0;
     }
   else
-    while (*p)
+    while (args != NULL && *args != '\0')
       {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be memory region numbers."));
-
-	num = atoi (p);
+	num = get_number_or_range (&args);
 	mem_disable (num);
-
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
       }
 }
 
@@ -679,15 +656,13 @@ mem_delete (int num)
 static void
 mem_delete_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
   int num;
 
   require_user_regions (from_tty);
 
   target_dcache_invalidate ();
 
-  if (p == 0)
+  if (args == NULL || *args == '\0')
     {
       if (query (_("Delete all memory regions? ")))
 	mem_clear ();
@@ -695,20 +670,10 @@ mem_delete_command (char *args, int from
       return;
     }
 
-  while (*p)
+  while (args != NULL && *args != '\0')
     {
-      p1 = p;
-      while (*p1 >= '0' && *p1 <= '9')
-	p1++;
-      if (*p1 && *p1 != ' ' && *p1 != '\t')
-	error (_("Arguments must be memory region numbers."));
-
-      num = atoi (p);
+      num = get_number_or_range (&args);
       mem_delete (num);
-
-      p = p1;
-      while (*p == ' ' || *p == '\t')
-	p++;
     }
 
   dont_repeat ();
@@ -739,19 +704,19 @@ where <mode>  may be rw (read/write), ro
   add_cmd ("mem", class_vars, mem_enable_command, _("\
 Enable memory region.\n\
 Arguments are the code numbers of the memory regions to enable.\n\
-Usage: enable mem <code number>\n\
+Usage: enable mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &enablelist);
 
   add_cmd ("mem", class_vars, mem_disable_command, _("\
 Disable memory region.\n\
 Arguments are the code numbers of the memory regions to disable.\n\
-Usage: disable mem <code number>\n\
+Usage: disable mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &disablelist);
 
   add_cmd ("mem", class_vars, mem_delete_command, _("\
 Delete memory region.\n\
 Arguments are the code numbers of the memory regions to delete.\n\
-Usage: delete mem <code number>\n\
+Usage: delete mem <code number>...\n\
 Do \"info mem\" to see current list of code numbers."), &deletelist);
 
   add_info ("mem", mem_info_command,
Index: testsuite/gdb.base/memattr.c
===================================================================
RCS file: testsuite/gdb.base/memattr.c
diff -N testsuite/gdb.base/memattr.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/memattr.c	23 Feb 2011 18:28:43 -0000
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define MEMSIZE 64
+static int mem1[MEMSIZE] = {111, 222, 333, 444, 555};
+static int mem2[MEMSIZE];
+static int mem3[MEMSIZE];
+static int mem4[MEMSIZE];
+static int mem5[MEMSIZE];
+
+int main()
+{
+  return 0;
+}
Index: testsuite/gdb.base/memattr.exp
===================================================================
RCS file: testsuite/gdb.base/memattr.exp
diff -N testsuite/gdb.base/memattr.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gdb.base/memattr.exp	23 Feb 2011 18:28:43 -0000
@@ -0,0 +1,455 @@
+# Copyright 2011
+# Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite
+
+# Test the memory attribute commands.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set testfile "memattr"
+set srcfile  ${testfile}.c
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile] } {
+    return -1
+}
+
+runto main
+
+set mem1start -1
+set mem2start -1
+set mem3start -1
+set mem4start -1
+set mem5start -1
+
+set mem1end -1
+set mem2end -1
+set mem3end -1
+set mem4end -1
+set mem5end -1
+
+
+gdb_test_multiple "info address mem1" "get address of mem1" {
+    -re "Symbol \"mem1\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem1start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem2" "get address of mem2" {
+    -re "Symbol \"mem2\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem2start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem3" "get address of mem3" {
+    -re "Symbol \"mem3\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem3start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem4" "get address of mem4" {
+    -re "Symbol \"mem4\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem4start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "info address mem5" "get address of mem5" {
+    -re "Symbol \"mem5\" is static storage at address ($hex).*$gdb_prompt $" {
+	set mem5start $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem1\[64\]" "get end of mem1" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem1end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem2\[64\]" "get end of mem2" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem2end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem3\[64\]" "get end of mem3" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem3end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem4\[64\]" "get end of mem4" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem4end $expect_out(1,string)
+    }
+}
+
+gdb_test_multiple "print &mem5\[64\]" "get end of mem5" {
+    -re "$decimal = .* ($hex).*$gdb_prompt $" {
+	set mem5end $expect_out(1,string)
+    }
+}
+
+gdb_test_no_output "mem $mem1start $mem1end wo" "create mem region 1"
+gdb_test_no_output "mem $mem2start $mem2end ro" "create mem region 2"
+gdb_test_no_output "mem $mem3start $mem3end rw" "create mem region 3"
+gdb_test_no_output "mem $mem4start $mem4end rw" "create mem region 4"
+gdb_test_no_output "mem $mem5start $mem5end rw" "create mem region 5"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "info mem(1)" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "info mem (1)"
+	} else {
+	    fail "info mem (1)"
+	}
+    }
+}
+
+#
+# Test read-only, write-only
+#
+
+# mem1 is write only: read should fail.
+gdb_test "print mem1\[1\]" \
+    "Cannot access memory at address $hex" \
+    "mem1 cannot be read"
+
+gdb_test "print mem1\[1\] = 9" \
+    "$decimal = 9" \
+    "mem1 can be written"
+
+# mem2 is read only: write should fail.
+gdb_test "print mem2\[1\] = 9" \
+    "Cannot access memory at address $hex" \
+    "mem2 cannot be written"
+
+gdb_test "print mem2\[1\]" \
+    "$decimal = 0" \
+    "mem2 can be read"
+
+#
+# Test disable and enable
+#
+
+gdb_test_no_output "disable mem 1" "disable mem 1"
+gdb_test "info mem" "1   n  .*" "mem 1 was disabled"
+
+gdb_test_no_output "enable mem 1" "enable mem 1"
+gdb_test "info mem" "1   y  .*" "mem 1 was enabled"
+
+gdb_test_no_output "disable mem 2 4"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 2 and 4 were disabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   n  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 2 and 4 were disabled"
+	} else {
+	    fail "mem 2 and 4 were disabled"
+	}
+    }
+}
+
+gdb_test_no_output "enable mem 2-4" "enable mem 2-4"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 2-4 were enabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 2-4 were enabled"
+	} else {
+	    fail "mem 2-4 were enabled"
+	}
+    }
+}
+
+gdb_test_no_output "disable mem" "disable mem"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 1 to 5 were disabled" {
+    -re "1   n  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   n  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   n  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   n  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 to 5 were disabled"
+	} else {
+	    fail "mem 1 to 5 were disabled"
+	}
+    }
+}
+
+gdb_test_no_output "enable mem" "enable mem"
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_multiple "info mem" "mem 1 to 5 were enabled" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { $see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 to 5 were enabled"
+	} else {
+	    fail "mem 1 to 5 were enabled"
+	}
+    }
+}
+
+gdb_test "disable mem 7 8" \
+    "No memory region number 7.*No memory region number 8." \
+    "disable non-existant regions"
+
+#
+# Test delete
+#
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_no_output "delete mem 1" "delete mem 1"
+gdb_test_multiple "info mem" "mem 1 was deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && $see2 && $see3 && $see4 && $see5 } then {
+	    pass "mem 1 was deleted"
+	} else {
+	    fail "mem 1 was deleted"
+	}
+    }
+}
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test_no_output "delete mem 2 4" "delete mem 2 4"
+gdb_test_multiple "info mem" "mem 2 and 4 were deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && !$see2 && $see3 && !$see4 && $see5 } then {
+	    pass "mem 2 and 4 were deleted"
+	} else {
+	    fail "mem 2 and 4 were deleted"
+	}
+    }
+}
+
+set see1 0
+set see2 0
+set see3 0
+set see4 0
+set see5 0
+
+gdb_test "delete mem 2-4" \
+    "No memory region number 2.*No memory region number 4." \
+    "delete mem 2-4"
+gdb_test_multiple "info mem" "mem 2-4 were deleted" {
+    -re "1   y  \t$hex $hex wo nocache \[^\r\n\]*" {
+	set see1 1
+	exp_continue
+    }
+    -re "2   y  \t$hex $hex ro nocache \[^\r\n\]*" {
+	set see2 1
+	exp_continue
+    }
+    -re "3   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see3 1
+	exp_continue
+    }
+    -re "4   y  \t$hex $hex rw nocache \[^\r\n\]*" {
+	set see4 1
+	exp_continue
+    }
+    -re "5   y  \t$hex $hex rw nocache .\[^\r\n\]*" {
+	set see5 1
+	exp_continue
+    }
+    -re "$gdb_prompt $" {
+	if { !$see1 && !$see2 && !$see3 && !$see4 && $see5 } then {
+	    pass "mem 2-4 were deleted"
+	} else {
+	    fail "mem 2-4 were deleted"
+	}
+    }
+}
+
+gdb_test "delete mem 8" "No memory region number 8." \
+    "delete non-existant region"