Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] new MI command for pattern filling of memory regions
@ 2012-05-09 16:18 Giuseppe MONTALTO
  2012-05-09 16:30 ` Tom Tromey
  2012-05-09 18:11 ` Tom Tromey
  0 siblings, 2 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-05-09 16:18 UTC (permalink / raw)
  To: gdb-patches

Hi all,

it's the first time I attempt a contribution to gdb, so feel free to ask for
details and, please, let me know if I'm doing anything wrong!

Short:
the proposed enhancement is about memory filling.

Rationale:
Filling a memory region with a pattern of bytes so far, can only be done by 
sending a potentially high number of repeated commands to gdb (depending on 
the size of the region that we need to fill).
If gdb is controlled through a GUI (e.g. Eclipse/CDT), this may cause the UI
to freeze for a quite long time. 
This is especially true when you're debugging a remote target, due to the
amount of generated network traffic.

Solution:
Implement an MI command that allows filling, with a specific pattern of bytes,
an arbitrarily sized memory region.

The proposed patch is here below, also including the related test script.

Regards,
	Giuseppe Montalto


From 9a168d60e69ce7618ab74591f230172554e4bc0b Mon Sep 17 00:00:00 2001
From: Giuseppe Montalto <giusepe.montalto@st.com>
Date: Wed, 9 May 2012 16:53:28 +0200
Subject: [PATCH] implemented a new MI command - data-fill-memory-bytes

---
 gdb/ChangeLog                           |    8 +++
 gdb/mi/mi-cmds.c                        |    1 +
 gdb/mi/mi-cmds.h                        |    1 +
 gdb/mi/mi-main.c                        |   73 +++++++++++++++++++++++++++++
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   76 +++++++++++++++++++++++++++++++
 6 files changed, 163 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-fill-memory.exp

diff --git gdb/ChangeLog gdb/ChangeLog
index 309473e..fe335bb 100644
--- gdb/ChangeLog
+++ gdb/ChangeLog
@@ -1,3 +1,11 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-cmds.h (mi_cmd_data_fill_memory_bytes): New Declaration
+	* mi/mi-cmds.c (data-fill-memory-bytes): New member in struct 
+	mi_cmd mi_cmds
+	* mi/mi-main.c  (mi_cmd_data_fill_memory_bytes): new Function
+	implementation
+
 2012-05-09  Pedro Alves  <palves@redhat.com>
 
 	* target.c (set_maintenance_target_async_permitted): Rename to ...
diff --git gdb/mi/mi-cmds.c gdb/mi/mi-cmds.c
index 9152489..020a483 100644
--- gdb/mi/mi-cmds.c
+++ gdb/mi/mi-cmds.c
@@ -47,6 +47,7 @@ struct mi_cmd mi_cmds[] =
   { "break-watch", { NULL, 0 }, mi_cmd_break_watch},
   { "data-disassemble", { NULL, 0 }, mi_cmd_disassemble},
   { "data-evaluate-expression", { NULL, 0 }, mi_cmd_data_evaluate_expression},
+  { "data-fill-memory-bytes", {NULL, 0}, mi_cmd_data_fill_memory_bytes},
   { "data-list-changed-registers", { NULL, 0 },
     mi_cmd_data_list_changed_registers},
   { "data-list-register-names", { NULL, 0 }, mi_cmd_data_list_register_names},
diff --git gdb/mi/mi-cmds.h gdb/mi/mi-cmds.h
index c8465f0..7a8756b 100644
--- gdb/mi/mi-cmds.h
+++ gdb/mi/mi-cmds.h
@@ -45,6 +45,7 @@ extern mi_cmd_argv_ftype mi_cmd_break_passcount;
 extern mi_cmd_argv_ftype mi_cmd_break_watch;
 extern mi_cmd_argv_ftype mi_cmd_disassemble;
 extern mi_cmd_argv_ftype mi_cmd_data_evaluate_expression;
+extern mi_cmd_argv_ftype mi_cmd_data_fill_memory_bytes;
 extern mi_cmd_argv_ftype mi_cmd_data_list_register_names;
 extern mi_cmd_argv_ftype mi_cmd_data_list_register_values;
 extern mi_cmd_argv_ftype mi_cmd_data_list_changed_registers;
diff --git gdb/mi/mi-main.c gdb/mi/mi-main.c
index 90fb624..5d94333 100644
--- gdb/mi/mi-main.c
+++ gdb/mi/mi-main.c
@@ -1693,6 +1693,79 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   do_cleanups (back_to);
 }
 
+/* DATA-MEMORY-FILL-RAW:
+
+   ADDR: start address
+   COUNT: number of bytes to be filled (decimal integer)
+   DATA: string of bytes to write from that address. (1,2,4 or 8 bytes) */
+void
+mi_cmd_data_fill_memory_bytes (char *command, char **argv, int argc)
+{
+  CORE_ADDR addr;
+  char *cdata;
+  gdb_byte *data;
+  int len, r, i, steps;
+  long int count, j;
+  struct cleanup *back_to;
+
+  if (argc != 3)
+    error (_("Usage: ADDR COUNT DATA."));
+
+  /* read parameters */
+  addr = parse_and_eval_address (argv[0]);
+  /* COUNT is supposed to be a decimal integer*/
+  count = strtol(argv[1], NULL, 10);
+  cdata = argv[2];
+
+  /* calculate and allocate a memory buffer for DATA parameter */
+  len = strlen (cdata);
+  
+  if ( len != 2 && len != 4 && len != 8 && len != 16 )
+	    error (_("DATA must be 2, 4, 8 or 16 characters."));
+  
+  len = len/2;
+  
+  if ( (long int)len < count )
+    {
+	  /* pattern is made of less bytes than count: 
+	     repeat pattern to fill memory */
+	  data = xmalloc (count/len);
+	  back_to = make_cleanup (xfree, data);
+	
+	  steps = count/len;
+	  for (j = 0; j < steps; j++)
+		{
+		  for (i = 0; i < len; ++i)
+			{
+			  int x;
+			  sscanf (cdata + i * 2, "%02x", &x);
+			  data[i + j * len] = (gdb_byte)x;
+			}
+		}
+    }
+  else
+    {
+	  /* pattern is longer than (or equal to) count: 
+	     just copy len bytes */
+	  data = xmalloc (len);
+	  back_to = make_cleanup (xfree, data);
+	
+	  for (i = 0; i < len; ++i)
+		{
+		  int x;
+		  sscanf (cdata + i * 2, "%02x", &x);
+		  data[i] = (gdb_byte)x;
+		}
+    }
+
+  r = target_write_memory (addr, data, count);
+
+  if (r != 0)
+    error (_("Could not fill memory range with supplied pattern"));
+
+  do_cleanups (back_to);
+}
+
 void
 mi_cmd_enable_timings (char *command, char **argv, int argc)
 {
diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
index 43549bf..a49274f 100644
--- gdb/testsuite/ChangeLog
+++ gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-05-08  Maciej W. Rozycki  <macro@codesourcery.com>
 
 	* gdb.mi/mi-var-display.exp: Check for the existence of $fp
diff --git gdb/testsuite/gdb.mi/mi-fill-memory.exp gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..40147e5
--- /dev/null
+++ gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,76 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+# Copyright (C) 2012 STMicroelectronics
+
+# 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/>.
+
+#
+# test basic Machine interface (MI) operations
+#
+# Verify that, using the MI, we can load a program and do
+# other basic things that are used by all test files through  mi_gdb_exit,
+# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
+# mi_gdb_load, so we can safely use those.
+#
+# The goal is not to test gdb functionality, which is done by other tests,
+# but the command syntax and correct output response to MI operations.
+# 
+# added for testing the -data-fill-memory-bytes MI command
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+set testfile "mi-read-memory"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
+     untested mi-read-memory.exp
+     return -1
+}
+
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-fill-memory-bytes" \
+	"1\\^error,msg=\"Usage: ADDR COUNT DATA\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-fill-memory-bytes 8" \
+	"2\\^error,msg=\"Usage: ADDR COUNT DATA\.\""\
+	"two arguments missing"
+
+mi_gdb_test "3-data-fill-memory-bytes 8 ab"\
+	"3\\^error,msg=\"Usage: ADDR COUNT DATA\.\""\
+	"one argument missing"
+
+mi_gdb_test "4-data-fill-memory-bytes \$pc 8 abc"\
+	"4\\^error,msg=\"DATA must be 2, 4, 8 or 16 characters\.\""\
+	"wrong data length"
+
+mi_gdb_test "5-data-fill-memory-bytes \$pc 8 ab"\
+	"5\\\^done" \
+	"memory successfully filled"
+
+mi_gdb_test "6-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit
+return 0
-- 
1.7.4.msysgit.0


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] new MI command for pattern filling of memory regions
  2012-05-09 16:18 [PATCH] new MI command for pattern filling of memory regions Giuseppe MONTALTO
@ 2012-05-09 16:30 ` Tom Tromey
  2012-05-09 18:11 ` Tom Tromey
  1 sibling, 0 replies; 37+ messages in thread
From: Tom Tromey @ 2012-05-09 16:30 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: gdb-patches

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> it's the first time I attempt a contribution to gdb, so feel
Giuseppe> free to ask for details and, please, let me know if I'm doing
Giuseppe> anything wrong!

Do you have a copyright assignment set up?
If not, let me know and we can get you started.
This must be completed before we can accept a patch.

I didn't review the patch.  I think it is probably better to get the
copyright stuff sorted out first.

I did notice that the patch doesn't include a documentation change.
All new MI commands must be documented.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] new MI command for pattern filling of memory regions
  2012-05-09 16:18 [PATCH] new MI command for pattern filling of memory regions Giuseppe MONTALTO
  2012-05-09 16:30 ` Tom Tromey
@ 2012-05-09 18:11 ` Tom Tromey
  2012-05-10 11:41   ` Giuseppe MONTALTO
  1 sibling, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-05-09 18:11 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: gdb-patches

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> it's the first time I attempt a contribution to gdb, so feel
Giuseppe> free to ask for details and, please, let me know if I'm doing
Giuseppe> anything wrong!

I was told off-list that you are covered by your employer's assignment.
So, good.

Giuseppe> the proposed enhancement is about memory filling.

Giuseppe> Filling a memory region with a pattern of bytes so far, can
Giuseppe> only be done by sending a potentially high number of repeated
Giuseppe> commands to gdb (depending on the size of the region that we
Giuseppe> need to fill).  If gdb is controlled through a GUI
Giuseppe> (e.g. Eclipse/CDT), this may cause the UI to freeze for a
Giuseppe> quite long time.  This is especially true when you're
Giuseppe> debugging a remote target, due to the amount of generated
Giuseppe> network traffic.

I started to review this, but I wondered why a new command is necessary.

It seems like you could send any hex string to want to
-data-write-memory-bytes.

Alternatively, if you really want the repeat count, why not just add an
optional argument to -data-write-memory-bytes?  That would seem to be
simpler.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] new MI command for pattern filling of memory regions
  2012-05-09 18:11 ` Tom Tromey
@ 2012-05-10 11:41   ` Giuseppe MONTALTO
  2012-05-10 13:57     ` Tom Tromey
  0 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-05-10 11:41 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches



> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, May 09, 2012 8:11 PM
> To: Giuseppe MONTALTO
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] new MI command for pattern filling of memory
> regions
> 
> I was told off-list that you are covered by your employer's assignment.
> So, good.

I was almost sure about that, though I had no way to check it.

> 
> Giuseppe> the proposed enhancement is about memory filling.
> 
> I started to review this, but I wondered why a new command is
> necessary.
> 
> It seems like you could send any hex string to want to
> -data-write-memory-bytes.
> 
> Alternatively, if you really want the repeat count, why not just add an
> optional argument to -data-write-memory-bytes?  That would seem to be
> simpler.

I didn't want to alter the syntax of an already existing command.

I did so, in the past, on a custom version of gdb, and this caused me a 
lot of trouble while migrating to a newer release: I discovered that 
the new release featured another argument to a command I had amended,
which was conflicting with my own changes, so I had to rework both!

Giuseppe



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] new MI command for pattern filling of memory regions
  2012-05-10 11:41   ` Giuseppe MONTALTO
@ 2012-05-10 13:57     ` Tom Tromey
  2012-05-11  8:53       ` Giuseppe MONTALTO
  0 siblings, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-05-10 13:57 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: gdb-patches

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> I did so, in the past, on a custom version of gdb, and this
Giuseppe> caused me a lot of trouble while migrating to a newer release:
Giuseppe> I discovered that the new release featured another argument to
Giuseppe> a command I had amended, which was conflicting with my own
Giuseppe> changes, so I had to rework both!

Now you're in the lucky position to add arguments and mess up other
people's private extensions :-)

Ok, seriously, I think adding an optional argument, if that is what you
need, is the better route.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] new MI command for pattern filling of memory regions
  2012-05-10 13:57     ` Tom Tromey
@ 2012-05-11  8:53       ` Giuseppe MONTALTO
  2012-05-11 14:36         ` Tom Tromey
  0 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-05-11  8:53 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

I guess you're right, so here below there is a brand new patch.

* No new MI functions
* Feature handled with an additional (optional) argument for "mi_cmd_data_write_memory()"
* New Expect test script (updated accordingly)

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Thursday, May 10, 2012 3:57 PM
> To: Giuseppe MONTALTO
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] new MI command for pattern filling of memory
> regions
> 
> 
> Ok, seriously, I think adding an optional argument, if that is what you
> need, is the better route.
> 
> Tom

Regards,
	Giuseppe

The patch:

 gdb/ChangeLog                           |    5 ++
 gdb/mi/mi-main.c                        |   53 +++++++++++++++++-----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   72 +++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 12 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 309473e..9ec8882 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
+	parameter for pattern filling of memory regions
+
 2012-05-09  Pedro Alves  <palves@redhat.com>
 
 	* target.c (set_maintenance_target_async_permitted): Rename to ...
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 90fb624..a811035 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1658,7 +1658,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1666,27 +1667,55 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  int len, r, i, steps;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+     count = strtol(argv[2], NULL, 10);
+  else
+	 count = (long int)len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
-
-  for (i = 0; i < len; ++i)
+  if ( (long int)len < count )
+    {
+	  /* pattern is made of less bytes than count: 
+	     repeat pattern to fill memory.  */
+	  data = xmalloc (count/len);
+	  back_to = make_cleanup (xfree, data);
+	
+	  steps = count/len;
+	  for (j = 0; j < steps; j++)
+		{
+		  for (i = 0; i < len; ++i)
+			{
+			  int x;
+			  sscanf (cdata + i * 2, "%02x", &x);
+			  data[i + j * len] = (gdb_byte)x;
+			}
+		}
+    }
+  else
     {
-      int x;
-      sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+	  /* pattern is longer than (or equal to) count: 
+	     just copy len bytes.  */
+	  data = xmalloc (len);
+	  back_to = make_cleanup (xfree, data);
+	
+	  for (i = 0; i < len; ++i)
+		{
+		  int x;
+		  sscanf (cdata + i * 2, "%02x", &x);
+		  data[i] = (gdb_byte)x;
+		}
     }
 
-  r = target_write_memory (addr, data, len);
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 43549bf..a49274f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-05-08  Maciej W. Rozycki  <macro@codesourcery.com>
 
 	* gdb.mi/mi-var-display.exp: Check for the existence of $fp
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..74c7682
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,72 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+# Copyright (C) 2012 STMicroelectronics
+
+# 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/>.
+
+#
+# test basic Machine interface (MI) operations
+#
+# Verify that, using the MI, we can load a program and do
+# other basic things that are used by all test files through  mi_gdb_exit,
+# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
+# mi_gdb_load, so we can safely use those.
+#
+# The goal is not to test gdb functionality, which is done by other tests,
+# but the command syntax and correct output response to MI operations.
+# 
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+set testfile "mi-read-memory"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
+     untested mi-read-memory.exp
+     return -1
+}
+
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes" \
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\"" \
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8" \
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\"" \
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab" \
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8" \
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "6-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit
+return 0


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] new MI command for pattern filling of memory regions
  2012-05-11  8:53       ` Giuseppe MONTALTO
@ 2012-05-11 14:36         ` Tom Tromey
  2012-05-11 16:06           ` Giuseppe MONTALTO
                             ` (2 more replies)
  0 siblings, 3 replies; 37+ messages in thread
From: Tom Tromey @ 2012-05-11 14:36 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: gdb-patches

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> +  if (argc == 3)
Giuseppe> +     count = strtol(argv[2], NULL, 10);
Giuseppe> +  else
Giuseppe> +	 count = (long int)len;

The indentation is wrong here and elsewhere in the patch.
There are missing spaces as well.
See the GNU Coding Standards for details.

I don't think the cast here, or other casts in the patch, are necessary.

Giuseppe> +	  /* pattern is made of less bytes than count: 
Giuseppe> +	     repeat pattern to fill memory.  */
Giuseppe> +	  data = xmalloc (count/len);

This allocates count/len bytes...

Giuseppe> +	  steps = count/len;
Giuseppe> +	  for (j = 0; j < steps; j++)
Giuseppe> +		{
Giuseppe> +		  for (i = 0; i < len; ++i)
Giuseppe> +			{
Giuseppe> +			  int x;
Giuseppe> +			  sscanf (cdata + i * 2, "%02x", &x);
Giuseppe> +			  data[i + j * len] = (gdb_byte)x;

... but writes 'count' bytes.

Re-invoking sscanf on each iteration seems odd to me.  I think it would
be cheaper to do this loop once and then memcpy the bits into place.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] new MI command for pattern filling of memory regions
  2012-05-11 14:36         ` Tom Tromey
@ 2012-05-11 16:06           ` Giuseppe MONTALTO
  2012-05-25 14:35           ` Giuseppe MONTALTO
  2012-06-12  9:07           ` Giuseppe MONTALTO
  2 siblings, 0 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-05-11 16:06 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

Thanks,

my answers are interleaved below.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Friday, May 11, 2012 4:14 PM
> To: Giuseppe MONTALTO
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] new MI command for pattern filling of memory
> regions
> 
> The indentation is wrong here and elsewhere in the patch.
> There are missing spaces as well.
> See the GNU Coding Standards for details.

I'll try to better format the code.

> 
> I don't think the cast here, or other casts in the patch, are
> necessary.

Consider them gone.

> 
> Giuseppe> +	  /* pattern is made of less bytes than count:
> Giuseppe> +	     repeat pattern to fill memory.  */
> Giuseppe> +	  data = xmalloc (count/len);
> 
> This allocates count/len bytes...
> 
> Giuseppe> +	  steps = count/len;
> Giuseppe> +	  for (j = 0; j < steps; j++)
> Giuseppe> +		{
> Giuseppe> +		  for (i = 0; i < len; ++i)
> Giuseppe> +			{
> Giuseppe> +			  int x;
> Giuseppe> +			  sscanf (cdata + i * 2, "%02x", &x);
> Giuseppe> +			  data[i + j * len] = (gdb_byte)x;
> 
> ... but writes 'count' bytes.

you're right; I was supposed to allocate 'count' bytes.

Incidentally, there's a worse error there: 
is that I'm counting the cycles basing on the result of an integer 
division, hence not taking into account the spare bytes!

I'll fix that too...

	Giuseppe


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] new MI command for pattern filling of memory regions
  2012-05-11 14:36         ` Tom Tromey
  2012-05-11 16:06           ` Giuseppe MONTALTO
@ 2012-05-25 14:35           ` Giuseppe MONTALTO
  2012-06-12  9:07           ` Giuseppe MONTALTO
  2 siblings, 0 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-05-25 14:35 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

I reworked the patch, including the test script;
Accordingly with your feedback, the sscanf cycle is now performed only once.

Here's the new patch, please let me know if there is anything more to change.

Regards,
	Giuseppe

gdb/ChangeLog                           |    5 ++
 gdb/mi/mi-main.c                        |   50 +++++++++++++++++----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   72 +++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 309473e..9ec8882 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
+	parameter for pattern filling of memory regions
+
 2012-05-09  Pedro Alves  <palves@redhat.com>
 
 	* target.c (set_maintenance_target_async_permitted): Rename to ...
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 90fb624..7315848 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1658,7 +1658,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1666,27 +1667,58 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  int len, r, i, steps, reminder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
-  len = strlen (cdata)/2;
+  len = strlen (cdata) / 2;
+  if (argc == 3)
+    count = strtol(argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof(gdb_byte));
 
   for (i = 0; i < len; ++i)
     {
       int x;
       sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      databuf[i] = (gdb_byte)x;
     }
 
-  r = target_write_memory (addr, data, len);
+  if (len < count)
+    {
+      /* pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      back_to = make_cleanup (xfree, data);
+    
+      steps = count / len;
+      reminder = count % len; /* there may be some spare bytes.  */
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, (size_t) len);
+
+      if (reminder > 0) /* copy spare bytes too.  */
+        memcpy (data + steps * len, databuf, (size_t) reminder);
+    }
+  else
+    {
+      /* pattern is longer than (or equal to) count: 
+         just copy len bytes.  */
+      data = xmalloc (len);
+      back_to = make_cleanup (xfree, data);
+      memcpy (data, databuf, (size_t) len);
+    }
+
+  xfree(databuf);
+
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 43549bf..a49274f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-05-08  Maciej W. Rozycki  <macro@codesourcery.com>
 
 	* gdb.mi/mi-var-display.exp: Check for the existence of $fp
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..b6961b1
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,72 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+# Copyright (C) 2012 STMicroelectronics
+
+# 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/>.
+
+#
+# test basic Machine interface (MI) operations
+#
+# Verify that, using the MI, we can load a program and do
+# other basic things that are used by all test files through  mi_gdb_exit,
+# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
+# mi_gdb_load, so we can safely use those.
+#
+# The goal is not to test gdb functionality, which is done by other tests,
+# but the command syntax and correct output response to MI operations.
+# 
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+set testfile "mi-read-memory"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
+     untested mi-read-memory.exp
+     return -1
+}
+
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit
+return 0



> -----Original Message-----
> From: Giuseppe MONTALTO
> Sent: Friday, May 11, 2012 6:06 PM
> To: 'Tom Tromey'
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH] new MI command for pattern filling of memory
> regions
> 
> Thanks,
> 
> my answers are interleaved below.
> 
> > -----Original Message-----
> > From: Tom Tromey [mailto:tromey@redhat.com]
> > Sent: Friday, May 11, 2012 4:14 PM
> > To: Giuseppe MONTALTO
> > Cc: gdb-patches@sourceware.org
> > Subject: Re: [PATCH] new MI command for pattern filling of memory
> > regions
> >
> > The indentation is wrong here and elsewhere in the patch.
> > There are missing spaces as well.
> > See the GNU Coding Standards for details.
> 
> I'll try to better format the code.
> 
> >
> > I don't think the cast here, or other casts in the patch, are
> > necessary.
> 
> Consider them gone.
> 
> >
> > Giuseppe> +	  /* pattern is made of less bytes than count:
> > Giuseppe> +	     repeat pattern to fill memory.  */
> > Giuseppe> +	  data = xmalloc (count/len);
> >
> > This allocates count/len bytes...
> >
> > Giuseppe> +	  steps = count/len;
> > Giuseppe> +	  for (j = 0; j < steps; j++)
> > Giuseppe> +		{
> > Giuseppe> +		  for (i = 0; i < len; ++i)
> > Giuseppe> +			{
> > Giuseppe> +			  int x;
> > Giuseppe> +			  sscanf (cdata + i * 2, "%02x", &x);
> > Giuseppe> +			  data[i + j * len] = (gdb_byte)x;
> >
> > ... but writes 'count' bytes.
> 
> you're right; I was supposed to allocate 'count' bytes.
> 
> Incidentally, there's a worse error there:
> is that I'm counting the cycles basing on the result of an integer
> division, hence not taking into account the spare bytes!
> 
> I'll fix that too...
> 
> 	Giuseppe


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] new MI command for pattern filling of memory regions
  2012-05-11 14:36         ` Tom Tromey
  2012-05-11 16:06           ` Giuseppe MONTALTO
  2012-05-25 14:35           ` Giuseppe MONTALTO
@ 2012-06-12  9:07           ` Giuseppe MONTALTO
  2012-06-12 10:01             ` Abid, Hafiz
  2 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-06-12  9:07 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches; +Cc: Giuseppe MONTALTO

Hi!

Sorry to bother you, but I received no feedback on the patch after three weeks; is there any chance 
for it to be reviewed on time for 7.5?

> -----Original Message-----
> From: Giuseppe MONTALTO
> Sent: Friday, May 25, 2012 4:35 PM
> To: 'Tom Tromey'
> Cc: 'gdb-patches@sourceware.org'
> Subject: RE: [PATCH] new MI command for pattern filling of memory
> regions
> 
> I reworked the patch, including the test script;
> Accordingly with your feedback, the sscanf cycle is now performed only
> once.
> 
> Here's the new patch, please let me know if there is anything more to
> change.
> 
> Regards,
> 	Giuseppe
> 
> gdb/ChangeLog                           |    5 ++
>  gdb/mi/mi-main.c                        |   50 +++++++++++++++++----
>  gdb/testsuite/ChangeLog                 |    4 ++
>  gdb/testsuite/gdb.mi/mi-fill-memory.exp |   72
> +++++++++++++++++++++++++++++++
>  4 files changed, 122 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 309473e..9ec8882 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
> +
> +	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
> +	parameter for pattern filling of memory regions
> +
>  2012-05-09  Pedro Alves  <palves@redhat.com>
> 
>  	* target.c (set_maintenance_target_async_permitted): Rename to
> ...
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 90fb624..7315848 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -1658,7 +1658,8 @@ mi_cmd_data_write_memory (char *command, char
> **argv, int argc)
>  /* Implementation of the -data-write-memory-bytes command.
> 
>     ADDR: start address
> -   DATA: string of bytes to write at that address.  */
> +   DATA: string of bytes to write at that address
> +   COUNT: number of bytes to be filled (decimal integer).  */
> 
>  void
>  mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
> @@ -1666,27 +1667,58 @@ mi_cmd_data_write_memory_bytes (char *command,
> char **argv, int argc)
>    CORE_ADDR addr;
>    char *cdata;
>    gdb_byte *data;
> -  int len, r, i;
> +  gdb_byte *databuf;
> +  int len, r, i, steps, reminder;
> +  long int count, j;
>    struct cleanup *back_to;
> 
> -  if (argc != 2)
> -    error (_("Usage: ADDR DATA."));
> +  if (argc != 2 && argc != 3)
> +    error (_("Usage: ADDR DATA [COUNT]."));
> 
>    addr = parse_and_eval_address (argv[0]);
>    cdata = argv[1];
> -  len = strlen (cdata)/2;
> +  len = strlen (cdata) / 2;
> +  if (argc == 3)
> +    count = strtol(argv[2], NULL, 10);
> +  else
> +    count = len;
> 
> -  data = xmalloc (len);
> -  back_to = make_cleanup (xfree, data);
> +  databuf = xmalloc (len * sizeof(gdb_byte));
> 
>    for (i = 0; i < len; ++i)
>      {
>        int x;
>        sscanf (cdata + i * 2, "%02x", &x);
> -      data[i] = (gdb_byte) x;
> +      databuf[i] = (gdb_byte)x;
>      }
> 
> -  r = target_write_memory (addr, data, len);
> +  if (len < count)
> +    {
> +      /* pattern is made of less bytes than count:
> +         repeat pattern to fill memory.  */
> +      data = xmalloc (count);
> +      back_to = make_cleanup (xfree, data);
> +
> +      steps = count / len;
> +      reminder = count % len; /* there may be some spare bytes.  */
> +      for (j = 0; j < steps; j++)
> +        memcpy (data + j * len, databuf, (size_t) len);
> +
> +      if (reminder > 0) /* copy spare bytes too.  */
> +        memcpy (data + steps * len, databuf, (size_t) reminder);
> +    }
> +  else
> +    {
> +      /* pattern is longer than (or equal to) count:
> +         just copy len bytes.  */
> +      data = xmalloc (len);
> +      back_to = make_cleanup (xfree, data);
> +      memcpy (data, databuf, (size_t) len);
> +    }
> +
> +  xfree(databuf);
> +
> +  r = target_write_memory (addr, data, count);
>    if (r != 0)
>      error (_("Could not write memory"));
> 
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 43549bf..a49274f 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
> +
> +	* gdb.mi/mi-fill-memory.exp: New test.
> +
>  2012-05-08  Maciej W. Rozycki  <macro@codesourcery.com>
> 
>  	* gdb.mi/mi-var-display.exp: Check for the existence of $fp
> diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> new file mode 100644
> index 0000000..b6961b1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> @@ -0,0 +1,72 @@
> +# Copyright (C) 2012 Free Software Foundation, Inc.
> +# Copyright (C) 2012 STMicroelectronics
> +
> +# 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/>.
> +
> +#
> +# test basic Machine interface (MI) operations
> +#
> +# Verify that, using the MI, we can load a program and do
> +# other basic things that are used by all test files through
> mi_gdb_exit,
> +# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
> +# mi_gdb_load, so we can safely use those.
> +#
> +# The goal is not to test gdb functionality, which is done by other
> tests,
> +# but the command syntax and correct output response to MI operations.
> +#
> +# added for testing the -data-write-memory-bytes MI command
> enhancements
> +#
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +set testfile "mi-read-memory"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable {debug additional_flags=-DFAKEARGV}] != "" } {
> +     untested mi-read-memory.exp
> +     return -1
> +}
> +
> +
> +mi_run_to_main
> +mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
> +
> +mi_gdb_test "1-data-write-memory-bytes"\
> +	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
> +	"no arguments"
> +
> +mi_gdb_test "2-data-write-memory-bytes 8"\
> +	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
> +	"one argument missing"
> +
> +mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
> +	"3\\\^done" \
> +	"memory successfully written"
> +
> +mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
> +	"4\\\^done" \
> +	"memory successfully filled (8 bytes)"
> +
> +mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
> +    ".*0xabababab.*" \
> +    "pattern correctly read from memory"
> +
> +mi_gdb_exit
> +return 0
> 
> 
> 
> > -----Original Message-----
> > From: Giuseppe MONTALTO
> > Sent: Friday, May 11, 2012 6:06 PM
> > To: 'Tom Tromey'
> > Cc: gdb-patches@sourceware.org
> > Subject: RE: [PATCH] new MI command for pattern filling of memory
> > regions
> >
> > Thanks,
> >
> > my answers are interleaved below.
> >
> > > -----Original Message-----
> > > From: Tom Tromey [mailto:tromey@redhat.com]
> > > Sent: Friday, May 11, 2012 4:14 PM
> > > To: Giuseppe MONTALTO
> > > Cc: gdb-patches@sourceware.org
> > > Subject: Re: [PATCH] new MI command for pattern filling of memory
> > > regions
> > >
> > > The indentation is wrong here and elsewhere in the patch.
> > > There are missing spaces as well.
> > > See the GNU Coding Standards for details.
> >
> > I'll try to better format the code.
> >
> > >
> > > I don't think the cast here, or other casts in the patch, are
> > > necessary.
> >
> > Consider them gone.
> >
> > >
> > > Giuseppe> +	  /* pattern is made of less bytes than count:
> > > Giuseppe> +	     repeat pattern to fill memory.  */
> > > Giuseppe> +	  data = xmalloc (count/len);
> > >
> > > This allocates count/len bytes...
> > >
> > > Giuseppe> +	  steps = count/len;
> > > Giuseppe> +	  for (j = 0; j < steps; j++)
> > > Giuseppe> +		{
> > > Giuseppe> +		  for (i = 0; i < len; ++i)
> > > Giuseppe> +			{
> > > Giuseppe> +			  int x;
> > > Giuseppe> +			  sscanf (cdata + i * 2, "%02x", &x);
> > > Giuseppe> +			  data[i + j * len] = (gdb_byte)x;
> > >
> > > ... but writes 'count' bytes.
> >
> > you're right; I was supposed to allocate 'count' bytes.
> >
> > Incidentally, there's a worse error there:
> > is that I'm counting the cycles basing on the result of an integer
> > division, hence not taking into account the spare bytes!
> >
> > I'll fix that too...
> >
> > 	Giuseppe


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] new MI command for pattern filling of memory regions
  2012-06-12  9:07           ` Giuseppe MONTALTO
@ 2012-06-12 10:01             ` Abid, Hafiz
  2012-06-12 14:13               ` Giuseppe MONTALTO
  2012-06-12 14:22               ` [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions) Giuseppe MONTALTO
  0 siblings, 2 replies; 37+ messages in thread
From: Abid, Hafiz @ 2012-06-12 10:01 UTC (permalink / raw)
  To: Giuseppe MONTALTO, Tom Tromey, gdb-patches

I don't know much about this area of the code. Just a formatting comment.


-----Original Message-----
From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Giuseppe MONTALTO
Sent: Tuesday, June 12, 2012 10:07 AM
To: Tom Tromey; gdb-patches@sourceware.org
Cc: Giuseppe MONTALTO
Subject: RE: [PATCH] new MI command for pattern filling of memory regions

Hi!

Sorry to bother you, but I received no feedback on the patch after three weeks; is there any chance 
for it to be reviewed on time for 7.5?

> -----Original Message-----
> From: Giuseppe MONTALTO
> Sent: Friday, May 25, 2012 4:35 PM
> To: 'Tom Tromey'
> Cc: 'gdb-patches@sourceware.org'
> Subject: RE: [PATCH] new MI command for pattern filling of memory
> regions
> 
> I reworked the patch, including the test script;
> Accordingly with your feedback, the sscanf cycle is now performed only
> once.
> 
> Here's the new patch, please let me know if there is anything more to
> change.
> 
> Regards,
> 	Giuseppe
> 
> gdb/ChangeLog                           |    5 ++
>  gdb/mi/mi-main.c                        |   50 +++++++++++++++++----
>  gdb/testsuite/ChangeLog                 |    4 ++
>  gdb/testsuite/gdb.mi/mi-fill-memory.exp |   72
> +++++++++++++++++++++++++++++++
>  4 files changed, 122 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 309473e..9ec8882 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
> +
> +	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
> +	parameter for pattern filling of memory regions
> +
>  2012-05-09  Pedro Alves  <palves@redhat.com>
> 
>  	* target.c (set_maintenance_target_async_permitted): Rename to
> ...
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 90fb624..7315848 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -1658,7 +1658,8 @@ mi_cmd_data_write_memory (char *command, char
> **argv, int argc)
>  /* Implementation of the -data-write-memory-bytes command.
> 
>     ADDR: start address
> -   DATA: string of bytes to write at that address.  */
> +   DATA: string of bytes to write at that address
> +   COUNT: number of bytes to be filled (decimal integer).  */
> 
>  void
>  mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
> @@ -1666,27 +1667,58 @@ mi_cmd_data_write_memory_bytes (char *command,
> char **argv, int argc)
>    CORE_ADDR addr;
>    char *cdata;
>    gdb_byte *data;
> -  int len, r, i;
> +  gdb_byte *databuf;
> +  int len, r, i, steps, reminder;
> +  long int count, j;
>    struct cleanup *back_to;
> 
> -  if (argc != 2)
> -    error (_("Usage: ADDR DATA."));
> +  if (argc != 2 && argc != 3)
> +    error (_("Usage: ADDR DATA [COUNT]."));
> 
>    addr = parse_and_eval_address (argv[0]);
>    cdata = argv[1];
> -  len = strlen (cdata)/2;
> +  len = strlen (cdata) / 2;
> +  if (argc == 3)
> +    count = strtol(argv[2], NULL, 10);
> +  else
> +    count = len;
> 
> -  data = xmalloc (len);
> -  back_to = make_cleanup (xfree, data);
> +  databuf = xmalloc (len * sizeof(gdb_byte));
> 
>    for (i = 0; i < len; ++i)
>      {
>        int x;
>        sscanf (cdata + i * 2, "%02x", &x);
> -      data[i] = (gdb_byte) x;
> +      databuf[i] = (gdb_byte)x;
>      }
> 
> -  r = target_write_memory (addr, data, len);
> +  if (len < count)
> +    {
> +      /* pattern is made of less bytes than count:
> +         repeat pattern to fill memory.  */
> +      data = xmalloc (count);
> +      back_to = make_cleanup (xfree, data);
> +
> +      steps = count / len;
> +      reminder = count % len; /* there may be some spare bytes.  */
> +      for (j = 0; j < steps; j++)
> +        memcpy (data + j * len, databuf, (size_t) len);
> +
> +      if (reminder > 0) /* copy spare bytes too.  */
> +        memcpy (data + steps * len, databuf, (size_t) reminder);
> +    }
> +  else
> +    {
> +      /* pattern is longer than (or equal to) count:
> +         just copy len bytes.  */
> +      data = xmalloc (len);
> +      back_to = make_cleanup (xfree, data);
> +      memcpy (data, databuf, (size_t) len);
> +    }
> +
> +  xfree(databuf);

Please add a space before the opening parenthesis. 
> +
> +  r = target_write_memory (addr, data, count);
>    if (r != 0)
>      error (_("Could not write memory"));
> 
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 43549bf..a49274f 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,7 @@
> +2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
> +
> +	* gdb.mi/mi-fill-memory.exp: New test.
> +
>  2012-05-08  Maciej W. Rozycki  <macro@codesourcery.com>
> 
>  	* gdb.mi/mi-var-display.exp: Check for the existence of $fp
> diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> new file mode 100644
> index 0000000..b6961b1
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> @@ -0,0 +1,72 @@
> +# Copyright (C) 2012 Free Software Foundation, Inc.
> +# Copyright (C) 2012 STMicroelectronics
> +
> +# 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/>.
> +
> +#
> +# test basic Machine interface (MI) operations
> +#
> +# Verify that, using the MI, we can load a program and do
> +# other basic things that are used by all test files through
> mi_gdb_exit,
> +# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
> +# mi_gdb_load, so we can safely use those.
> +#
> +# The goal is not to test gdb functionality, which is done by other
> tests,
> +# but the command syntax and correct output response to MI operations.
> +#
> +# added for testing the -data-write-memory-bytes MI command
> enhancements
> +#
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +set testfile "mi-read-memory"
> +set srcfile ${testfile}.c
> +set binfile ${objdir}/${subdir}/${testfile}
> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable {debug additional_flags=-DFAKEARGV}] != "" } {
> +     untested mi-read-memory.exp
> +     return -1
> +}
> +
> +
> +mi_run_to_main
> +mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
> +
> +mi_gdb_test "1-data-write-memory-bytes"\
> +	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
> +	"no arguments"
> +
> +mi_gdb_test "2-data-write-memory-bytes 8"\
> +	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
> +	"one argument missing"
> +
> +mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
> +	"3\\\^done" \
> +	"memory successfully written"
> +
> +mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
> +	"4\\\^done" \
> +	"memory successfully filled (8 bytes)"
> +
> +mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
> +    ".*0xabababab.*" \
> +    "pattern correctly read from memory"
> +
> +mi_gdb_exit
> +return 0
> 
> 
> 
> > -----Original Message-----
> > From: Giuseppe MONTALTO
> > Sent: Friday, May 11, 2012 6:06 PM
> > To: 'Tom Tromey'
> > Cc: gdb-patches@sourceware.org
> > Subject: RE: [PATCH] new MI command for pattern filling of memory
> > regions
> >
> > Thanks,
> >
> > my answers are interleaved below.
> >
> > > -----Original Message-----
> > > From: Tom Tromey [mailto:tromey@redhat.com]
> > > Sent: Friday, May 11, 2012 4:14 PM
> > > To: Giuseppe MONTALTO
> > > Cc: gdb-patches@sourceware.org
> > > Subject: Re: [PATCH] new MI command for pattern filling of memory
> > > regions
> > >
> > > The indentation is wrong here and elsewhere in the patch.
> > > There are missing spaces as well.
> > > See the GNU Coding Standards for details.
> >
> > I'll try to better format the code.
> >
> > >
> > > I don't think the cast here, or other casts in the patch, are
> > > necessary.
> >
> > Consider them gone.
> >
> > >
> > > Giuseppe> +	  /* pattern is made of less bytes than count:
> > > Giuseppe> +	     repeat pattern to fill memory.  */
> > > Giuseppe> +	  data = xmalloc (count/len);
> > >
> > > This allocates count/len bytes...
> > >
> > > Giuseppe> +	  steps = count/len;
> > > Giuseppe> +	  for (j = 0; j < steps; j++)
> > > Giuseppe> +		{
> > > Giuseppe> +		  for (i = 0; i < len; ++i)
> > > Giuseppe> +			{
> > > Giuseppe> +			  int x;
> > > Giuseppe> +			  sscanf (cdata + i * 2, "%02x", &x);
> > > Giuseppe> +			  data[i + j * len] = (gdb_byte)x;
> > >
> > > ... but writes 'count' bytes.
> >
> > you're right; I was supposed to allocate 'count' bytes.
> >
> > Incidentally, there's a worse error there:
> > is that I'm counting the cycles basing on the result of an integer
> > division, hence not taking into account the spare bytes!
> >
> > I'll fix that too...
> >
> > 	Giuseppe


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] new MI command for pattern filling of memory regions
  2012-06-12 10:01             ` Abid, Hafiz
@ 2012-06-12 14:13               ` Giuseppe MONTALTO
  2012-06-12 14:22               ` [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions) Giuseppe MONTALTO
  1 sibling, 0 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-06-12 14:13 UTC (permalink / raw)
  To: Abid, Hafiz, Tom Tromey, gdb-patches

Thanks.

Incidentally, while fixing it I found another couple of similar formatting issues.

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Abid, Hafiz
> Sent: Tuesday, June 12, 2012 12:01 PM
> To: Giuseppe MONTALTO; Tom Tromey; gdb-patches@sourceware.org
> Subject: RE: [PATCH] new MI command for pattern filling of memory
> regions
> 
> I don't know much about this area of the code. Just a formatting
> comment.
> 
> 
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Giuseppe MONTALTO
> Sent: Tuesday, June 12, 2012 10:07 AM
> To: Tom Tromey; gdb-patches@sourceware.org
> Cc: Giuseppe MONTALTO
> Subject: RE: [PATCH] new MI command for pattern filling of memory
> regions
> 
> Hi!
> 
> Sorry to bother you, but I received no feedback on the patch after
> three weeks; is there any chance
> for it to be reviewed on time for 7.5?
> 
> > -----Original Message-----
> > From: Giuseppe MONTALTO
> > Sent: Friday, May 25, 2012 4:35 PM
> > To: 'Tom Tromey'
> > Cc: 'gdb-patches@sourceware.org'
> > Subject: RE: [PATCH] new MI command for pattern filling of memory
> > regions
> >
> > I reworked the patch, including the test script;
> > Accordingly with your feedback, the sscanf cycle is now performed
> only
> > once.
> >
> > Here's the new patch, please let me know if there is anything more to
> > change.
> >
> > Regards,
> > 	Giuseppe
> >
> > gdb/ChangeLog                           |    5 ++
> >  gdb/mi/mi-main.c                        |   50 +++++++++++++++++----
> >  gdb/testsuite/ChangeLog                 |    4 ++
> >  gdb/testsuite/gdb.mi/mi-fill-memory.exp |   72
> > +++++++++++++++++++++++++++++++
> >  4 files changed, 122 insertions(+), 9 deletions(-)
> >
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> > index 309473e..9ec8882 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,3 +1,8 @@
> > +2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
> > +
> > +	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
> > +	parameter for pattern filling of memory regions
> > +
> >  2012-05-09  Pedro Alves  <palves@redhat.com>
> >
> >  	* target.c (set_maintenance_target_async_permitted): Rename to
> > ...
> > diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> > index 90fb624..7315848 100644
> > --- a/gdb/mi/mi-main.c
> > +++ b/gdb/mi/mi-main.c
> > @@ -1658,7 +1658,8 @@ mi_cmd_data_write_memory (char *command, char
> > **argv, int argc)
> >  /* Implementation of the -data-write-memory-bytes command.
> >
> >     ADDR: start address
> > -   DATA: string of bytes to write at that address.  */
> > +   DATA: string of bytes to write at that address
> > +   COUNT: number of bytes to be filled (decimal integer).  */
> >
> >  void
> >  mi_cmd_data_write_memory_bytes (char *command, char **argv, int
> argc)
> > @@ -1666,27 +1667,58 @@ mi_cmd_data_write_memory_bytes (char
> *command,
> > char **argv, int argc)
> >    CORE_ADDR addr;
> >    char *cdata;
> >    gdb_byte *data;
> > -  int len, r, i;
> > +  gdb_byte *databuf;
> > +  int len, r, i, steps, reminder;
> > +  long int count, j;
> >    struct cleanup *back_to;
> >
> > -  if (argc != 2)
> > -    error (_("Usage: ADDR DATA."));
> > +  if (argc != 2 && argc != 3)
> > +    error (_("Usage: ADDR DATA [COUNT]."));
> >
> >    addr = parse_and_eval_address (argv[0]);
> >    cdata = argv[1];
> > -  len = strlen (cdata)/2;
> > +  len = strlen (cdata) / 2;
> > +  if (argc == 3)
> > +    count = strtol(argv[2], NULL, 10);
> > +  else
> > +    count = len;
> >
> > -  data = xmalloc (len);
> > -  back_to = make_cleanup (xfree, data);
> > +  databuf = xmalloc (len * sizeof(gdb_byte));
> >
> >    for (i = 0; i < len; ++i)
> >      {
> >        int x;
> >        sscanf (cdata + i * 2, "%02x", &x);
> > -      data[i] = (gdb_byte) x;
> > +      databuf[i] = (gdb_byte)x;
> >      }
> >
> > -  r = target_write_memory (addr, data, len);
> > +  if (len < count)
> > +    {
> > +      /* pattern is made of less bytes than count:
> > +         repeat pattern to fill memory.  */
> > +      data = xmalloc (count);
> > +      back_to = make_cleanup (xfree, data);
> > +
> > +      steps = count / len;
> > +      reminder = count % len; /* there may be some spare bytes.  */
> > +      for (j = 0; j < steps; j++)
> > +        memcpy (data + j * len, databuf, (size_t) len);
> > +
> > +      if (reminder > 0) /* copy spare bytes too.  */
> > +        memcpy (data + steps * len, databuf, (size_t) reminder);
> > +    }
> > +  else
> > +    {
> > +      /* pattern is longer than (or equal to) count:
> > +         just copy len bytes.  */
> > +      data = xmalloc (len);
> > +      back_to = make_cleanup (xfree, data);
> > +      memcpy (data, databuf, (size_t) len);
> > +    }
> > +
> > +  xfree(databuf);
> 
> Please add a space before the opening parenthesis.
> > +
> > +  r = target_write_memory (addr, data, count);
> >    if (r != 0)
> >      error (_("Could not write memory"));
> >
> > diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> > index 43549bf..a49274f 100644
> > --- a/gdb/testsuite/ChangeLog
> > +++ b/gdb/testsuite/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
> > +
> > +	* gdb.mi/mi-fill-memory.exp: New test.
> > +
> >  2012-05-08  Maciej W. Rozycki  <macro@codesourcery.com>
> >
> >  	* gdb.mi/mi-var-display.exp: Check for the existence of $fp
> > diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> > b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> > new file mode 100644
> > index 0000000..b6961b1
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> > @@ -0,0 +1,72 @@
> > +# Copyright (C) 2012 Free Software Foundation, Inc.
> > +# Copyright (C) 2012 STMicroelectronics
> > +
> > +# 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/>.
> > +
> > +#
> > +# test basic Machine interface (MI) operations
> > +#
> > +# Verify that, using the MI, we can load a program and do
> > +# other basic things that are used by all test files through
> > mi_gdb_exit,
> > +# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
> > +# mi_gdb_load, so we can safely use those.
> > +#
> > +# The goal is not to test gdb functionality, which is done by other
> > tests,
> > +# but the command syntax and correct output response to MI
> operations.
> > +#
> > +# added for testing the -data-write-memory-bytes MI command
> > enhancements
> > +#
> > +
> > +load_lib mi-support.exp
> > +set MIFLAGS "-i=mi"
> > +
> > +gdb_exit
> > +if [mi_gdb_start] {
> > +    continue
> > +}
> > +
> > +set testfile "mi-read-memory"
> > +set srcfile ${testfile}.c
> > +set binfile ${objdir}/${subdir}/${testfile}
> > +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> > executable {debug additional_flags=-DFAKEARGV}] != "" } {
> > +     untested mi-read-memory.exp
> > +     return -1
> > +}
> > +
> > +
> > +mi_run_to_main
> > +mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
> > +
> > +mi_gdb_test "1-data-write-memory-bytes"\
> > +	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
> > +	"no arguments"
> > +
> > +mi_gdb_test "2-data-write-memory-bytes 8"\
> > +	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
> > +	"one argument missing"
> > +
> > +mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
> > +	"3\\\^done" \
> > +	"memory successfully written"
> > +
> > +mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
> > +	"4\\\^done" \
> > +	"memory successfully filled (8 bytes)"
> > +
> > +mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
> > +    ".*0xabababab.*" \
> > +    "pattern correctly read from memory"
> > +
> > +mi_gdb_exit
> > +return 0
> >
> >
> >
> > > -----Original Message-----
> > > From: Giuseppe MONTALTO
> > > Sent: Friday, May 11, 2012 6:06 PM
> > > To: 'Tom Tromey'
> > > Cc: gdb-patches@sourceware.org
> > > Subject: RE: [PATCH] new MI command for pattern filling of memory
> > > regions
> > >
> > > Thanks,
> > >
> > > my answers are interleaved below.
> > >
> > > > -----Original Message-----
> > > > From: Tom Tromey [mailto:tromey@redhat.com]
> > > > Sent: Friday, May 11, 2012 4:14 PM
> > > > To: Giuseppe MONTALTO
> > > > Cc: gdb-patches@sourceware.org
> > > > Subject: Re: [PATCH] new MI command for pattern filling of memory
> > > > regions
> > > >
> > > > The indentation is wrong here and elsewhere in the patch.
> > > > There are missing spaces as well.
> > > > See the GNU Coding Standards for details.
> > >
> > > I'll try to better format the code.
> > >
> > > >
> > > > I don't think the cast here, or other casts in the patch, are
> > > > necessary.
> > >
> > > Consider them gone.
> > >
> > > >
> > > > Giuseppe> +	  /* pattern is made of less bytes than count:
> > > > Giuseppe> +	     repeat pattern to fill memory.  */
> > > > Giuseppe> +	  data = xmalloc (count/len);
> > > >
> > > > This allocates count/len bytes...
> > > >
> > > > Giuseppe> +	  steps = count/len;
> > > > Giuseppe> +	  for (j = 0; j < steps; j++)
> > > > Giuseppe> +		{
> > > > Giuseppe> +		  for (i = 0; i < len; ++i)
> > > > Giuseppe> +			{
> > > > Giuseppe> +			  int x;
> > > > Giuseppe> +			  sscanf (cdata + i * 2, "%02x", &x);
> > > > Giuseppe> +			  data[i + j * len] = (gdb_byte)x;
> > > >
> > > > ... but writes 'count' bytes.
> > >
> > > you're right; I was supposed to allocate 'count' bytes.
> > >
> > > Incidentally, there's a worse error there:
> > > is that I'm counting the cycles basing on the result of an integer
> > > division, hence not taking into account the spare bytes!
> > >
> > > I'll fix that too...
> > >
> > > 	Giuseppe


^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-06-12 10:01             ` Abid, Hafiz
  2012-06-12 14:13               ` Giuseppe MONTALTO
@ 2012-06-12 14:22               ` Giuseppe MONTALTO
  2012-09-14 16:30                 ` Tom Tromey
  1 sibling, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-06-12 14:22 UTC (permalink / raw)
  To: Abid, Hafiz, Tom Tromey, gdb-patches

New version of the patch.

 gdb/ChangeLog                           |    5 ++
 gdb/mi/mi-main.c                        |   50 +++++++++++++++++----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   72 +++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 309473e..9ec8882 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
+	parameter for pattern filling of memory regions
+
 2012-05-09  Pedro Alves  <palves@redhat.com>
 
 	* target.c (set_maintenance_target_async_permitted): Rename to ...
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 90fb624..1e8fca6 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1658,7 +1658,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1666,27 +1667,58 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  int len, r, i, steps, reminder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
-  len = strlen (cdata)/2;
+  len = strlen (cdata) / 2;
+  if (argc == 3)
+    count = strtol (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
 
   for (i = 0; i < len; ++i)
     {
       int x;
       sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      databuf[i] = (gdb_byte) x;
     }
 
-  r = target_write_memory (addr, data, len);
+  if (len < count)
+    {
+      /* pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      back_to = make_cleanup (xfree, data);
+    
+      steps = count / len;
+      reminder = count % len; /* there may be some spare bytes.  */
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, (size_t) len);
+
+      if (reminder > 0) /* copy spare bytes too.  */
+        memcpy (data + steps * len, databuf, (size_t) reminder);
+    }
+  else
+    {
+      /* pattern is longer than (or equal to) count: 
+         just copy len bytes.  */
+      data = xmalloc (len);
+      back_to = make_cleanup (xfree, data);
+      memcpy (data, databuf, (size_t) len);
+    }
+
+  xfree (databuf);
+
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 43549bf..a49274f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-05-09  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-05-08  Maciej W. Rozycki  <macro@codesourcery.com>
 
 	* gdb.mi/mi-var-display.exp: Check for the existence of $fp
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..b6961b1
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,72 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+# Copyright (C) 2012 STMicroelectronics
+
+# 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/>.
+
+#
+# test basic Machine interface (MI) operations
+#
+# Verify that, using the MI, we can load a program and do
+# other basic things that are used by all test files through  mi_gdb_exit,
+# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
+# mi_gdb_load, so we can safely use those.
+#
+# The goal is not to test gdb functionality, which is done by other tests,
+# but the command syntax and correct output response to MI operations.
+# 
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+set testfile "mi-read-memory"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
+     untested mi-read-memory.exp
+     return -1
+}
+
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit
+return 0


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-06-12 14:22               ` [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions) Giuseppe MONTALTO
@ 2012-09-14 16:30                 ` Tom Tromey
  2012-09-18 15:19                   ` Giuseppe MONTALTO
  0 siblings, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-09-14 16:30 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Abid, Hafiz, gdb-patches

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> New version of the patch.

I'm sorry about the delay on this.

I know it feels uncomfortable, but it really is good to ping patches --
say every week or so.  Otherwise they tend to slide off the radar.

Giuseppe> +  int len, r, i, steps, reminder;
Giuseppe> +  long int count, j;

I think now size_t is preferred.  (This changed since your patch
submission.)

Typo: should be "remainder", not "reminder".

Giuseppe> +    count = strtol (argv[2], NULL, 10);

I think strtoul instead.

Giuseppe> +  if (len < count)
Giuseppe> +    {
Giuseppe> +      /* pattern is made of less bytes than count: 
Giuseppe> +         repeat pattern to fill memory.  */

Comments should start with an uppercase letter.

Giuseppe> +  else
Giuseppe> +    {
Giuseppe> +      /* pattern is longer than (or equal to) count: 
Giuseppe> +         just copy len bytes.  */

Likewise.

Giuseppe> +  xfree (databuf);

It seems like you could do less work for the usual case where len==count.
That is, avoid the extra malloc and memcpy.

Giuseppe> +set testfile "mi-read-memory"
Giuseppe> +set srcfile ${testfile}.c
Giuseppe> +set binfile ${objdir}/${subdir}/${testfile}

Use standard_testfile now.

Giuseppe> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug additional_flags=-DFAKEARGV}] != "" } {
Giuseppe> +     untested mi-read-memory.exp

Use build_executable instead and don't call untested.

Giuseppe> +return 0

No need for a return at the end of a .exp file.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-09-14 16:30                 ` Tom Tromey
@ 2012-09-18 15:19                   ` Giuseppe MONTALTO
  2012-09-26 20:44                     ` Tom Tromey
  0 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-09-18 15:19 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Abid, Hafiz, gdb-patches

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

Hi Tom,

Thanks for your comments: I've reworked the patch taking them into account.
I have also updated it, so that it should now fit the current HEAD 

Please, find it attached.

Regards,
	Giuseppe.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Friday, September 14, 2012 6:31 PM
> To: Giuseppe MONTALTO
> Cc: Abid, Hafiz; gdb-patches@sourceware.org
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> >>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com>
> writes:
> 
> Giuseppe> New version of the patch.
> 
> I'm sorry about the delay on this.
> 
> I know it feels uncomfortable, but it really is good to ping patches -- say every
> week or so.  Otherwise they tend to slide off the radar.
> 
> Giuseppe> +  int len, r, i, steps, reminder;  long int count, j;
> 
> I think now size_t is preferred.  (This changed since your patch
> submission.)
> 
> Typo: should be "remainder", not "reminder".
> 
> Giuseppe> +    count = strtol (argv[2], NULL, 10);
> 
> I think strtoul instead.
> 
> Giuseppe> +  if (len < count)
> Giuseppe> +    {
> Giuseppe> +      /* pattern is made of less bytes than count:
> Giuseppe> +         repeat pattern to fill memory.  */
> 
> Comments should start with an uppercase letter.
> 
> Giuseppe> +  else
> Giuseppe> +    {
> Giuseppe> +      /* pattern is longer than (or equal to) count:
> Giuseppe> +         just copy len bytes.  */
> 
> Likewise.
> 
> Giuseppe> +  xfree (databuf);
> 
> It seems like you could do less work for the usual case where len==count.
> That is, avoid the extra malloc and memcpy.
> 
> Giuseppe> +set testfile "mi-read-memory"
> Giuseppe> +set srcfile ${testfile}.c
> Giuseppe> +set binfile ${objdir}/${subdir}/${testfile}
> 
> Use standard_testfile now.
> 
> Giuseppe> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}"
> executable {debug additional_flags=-DFAKEARGV}] != "" } {
> Giuseppe> +     untested mi-read-memory.exp
> 
> Use build_executable instead and don't call untested.
> 
> Giuseppe> +return 0
> 
> No need for a return at the end of a .exp file.
> 
> Tom

[-- Attachment #2: mi_cmd_data_write_memory_bytes.patch --]
[-- Type: application/octet-stream, Size: 6806 bytes --]

 gdb/ChangeLog                           |    5 ++
 gdb/mi/mi-main.c                        |   61 ++++++++++++++++++++++-----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   68 +++++++++++++++++++++++++++++++
 4 files changed, 127 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4ab15c4..3a0268e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-09-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
+	parameter for pattern filling of memory regions
+
 2012-09-17  Mike Wrighton  <wrighton@codesourcery.com>
 
 	* MAINTAINERS (Write After Approval): Add "Mike Wrighton".
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f1d21bc..5f670c2 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,30 +1665,68 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
 
   for (i = 0; i < len; ++i)
     {
       int x;
       sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      databuf[i] = (gdb_byte) x;
     }
 
-  r = target_write_memory (addr, data, len);
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      back_to = make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len; /* there may be some spare bytes.  */
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0) /* copy spare bytes too.  */
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else if (len > count)
+    {
+      /* Pattern is longer than count: 
+         just copy len bytes.  */
+      data = xmalloc (len);
+      back_to = make_cleanup (xfree, data);
+      memcpy (data, databuf, len);
+    }
+  else
+    {
+      /* Pattern is equal to count: 
+         we can just use databuf.  */
+      back_to = make_cleanup (xfree, data);
+      data = databuf;
+    }
+
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
+  xfree (databuf);
+
   do_cleanups (back_to);
 }
 
@@ -2097,10 +2136,10 @@ mi_cmd_execute (struct mi_parse *parse)
 
   current_context = parse;
 
-  if (parse->cmd->suppress_notification != NULL)
+  if (strncmp (parse->command, "break-", sizeof ("break-") - 1 ) == 0)
     {
-      make_cleanup_restore_integer (parse->cmd->suppress_notification);
-      *parse->cmd->suppress_notification = 1;
+      make_cleanup_restore_integer (&mi_suppress_breakpoint_notifications);
+      mi_suppress_breakpoint_notifications = 1;
     }
 
   if (parse->cmd->argv_func != NULL)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index b458aba..42f66be 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-09-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-09-17  Yao Qi  <yao@codesourcery.com>
 
 	* gdb.base/list.exp (set_listsize): Don't set arg to "unlimited"
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..3df6a96
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,68 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+# Copyright (C) 2012 STMicroelectronics
+
+# 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/>.
+
+#
+# test basic Machine interface (MI) operations
+#
+# Verify that, using the MI, we can load a program and do
+# other basic things that are used by all test files through  mi_gdb_exit,
+# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
+# mi_gdb_load, so we can safely use those.
+#
+# The goal is not to test gdb functionality, which is done by other tests,
+# but the command syntax and correct output response to MI operations.
+# 
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if {[build_executable ${testfile}.exp ${binfile} ${srcfile}.c {debug additional_flags=-DFAKEARGV}] == -1} {
+    return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-09-18 15:19                   ` Giuseppe MONTALTO
@ 2012-09-26 20:44                     ` Tom Tromey
  2012-09-27 15:27                       ` Giuseppe MONTALTO
  0 siblings, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-09-26 20:44 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Abid, Hafiz, gdb-patches

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> Thanks for your comments: I've reworked the patch taking them
Giuseppe> into account.  I have also updated it, so that it should now
Giuseppe> fit the current HEAD

Thanks.

Giuseppe> +	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
Giuseppe> +	parameter for pattern filling of memory regions

Capital "A" in "additional" here.

Your patch has a ^M character on each line...

Giuseppe> +  else
Giuseppe> +    {
Giuseppe> +      /* Pattern is equal to count: 
Giuseppe> +         we can just use databuf.  */
Giuseppe> +      back_to = make_cleanup (xfree, data);
Giuseppe> +      data = databuf;

This makes a cleanup referencing a garbage value of 'data' -- before
'data' is assigned.

Giuseppe> +  r = target_write_memory (addr, data, count);

There was a recent patch to change this line, so you may need to rebase
again.

Giuseppe> +  xfree (databuf);

If you fix the above cleanup issue, then this will double-free databuf.

Giuseppe> -  if (parse->cmd->suppress_notification != NULL)
Giuseppe> +  if (strncmp (parse->command, "break-", sizeof ("break-") - 1 ) == 0)
Giuseppe>      {
Giuseppe> -      make_cleanup_restore_integer (parse->cmd->suppress_notification);
Giuseppe> -      *parse->cmd->suppress_notification = 1;
Giuseppe> +      make_cleanup_restore_integer (&mi_suppress_breakpoint_notifications);
Giuseppe> +      mi_suppress_breakpoint_notifications = 1;
Giuseppe>      }

This looks like an unintentional change.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-09-26 20:44                     ` Tom Tromey
@ 2012-09-27 15:27                       ` Giuseppe MONTALTO
  2012-09-28 20:07                         ` Tom Tromey
  2012-10-18 15:41                         ` Pedro Alves
  0 siblings, 2 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-09-27 15:27 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches; +Cc: Abid, Hafiz

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

Thanks for your comments ,

the attached patch should fix all the issues.
Incidentally, I rebased as suggested, but found no issues with respect to:

    r = target_write_memory (addr, data, count);

regards,
	Giuseppe

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, September 26, 2012 10:44 PM
> To: Giuseppe MONTALTO
> Cc: Abid, Hafiz; gdb-patches@sourceware.org
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> >>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com>
> writes:
> 
> Giuseppe> Thanks for your comments: I've reworked the patch taking them
> Giuseppe> into account.  I have also updated it, so that it should now
> Giuseppe> fit the current HEAD
> 
> Thanks.
> 
> Giuseppe> +	* mi/mi-main.c  (mi_cmd_data_write_memory): additional
> Giuseppe> +	parameter for pattern filling of memory regions
> 
> Capital "A" in "additional" here.
> 
> Your patch has a ^M character on each line...
> 
> Giuseppe> +  else
> Giuseppe> +    {
> Giuseppe> +      /* Pattern is equal to count:
> Giuseppe> +         we can just use databuf.  */
> Giuseppe> +      back_to = make_cleanup (xfree, data);
> Giuseppe> +      data = databuf;
> 
> This makes a cleanup referencing a garbage value of 'data' -- before 'data' is
> assigned.
> 
> Giuseppe> +  r = target_write_memory (addr, data, count);
> 
> There was a recent patch to change this line, so you may need to rebase
> again.
> 
> Giuseppe> +  xfree (databuf);
> 
> If you fix the above cleanup issue, then this will double-free databuf.
> 
> Giuseppe> -  if (parse->cmd->suppress_notification != NULL)
> Giuseppe> +  if (strncmp (parse->command, "break-", sizeof ("break-") -
> Giuseppe> + 1 ) == 0)
> Giuseppe>      {
> Giuseppe> -      make_cleanup_restore_integer (parse->cmd-
> >suppress_notification);
> Giuseppe> -      *parse->cmd->suppress_notification = 1;
> Giuseppe> +      make_cleanup_restore_integer
> (&mi_suppress_breakpoint_notifications);
> Giuseppe> +      mi_suppress_breakpoint_notifications = 1;
> Giuseppe>      }
> 
> This looks like an unintentional change.
> 
> Tom

[-- Attachment #2: mi-fill-memory.patch --]
[-- Type: application/octet-stream, Size: 6194 bytes --]

From 9c0f768ed1e788bd35138febc4c8ac12d2ecffd4 Mon Sep 17 00:00:00 2001
From: Giuseppe Montalto <giuseppe.montalto@st.com>
Date: Thu, 27 Sep 2012 11:44:10 +0200
Subject: [PATCH] patch V.6

---
 gdb/ChangeLog                           |    5 ++
 gdb/mi/mi-main.c                        |   53 ++++++++++++++++++++----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   68 +++++++++++++++++++++++++++++++
 4 files changed, 122 insertions(+), 8 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-fill-memory.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 959e90d..060cb93 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-09-27  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c  (mi_cmd_data_write_memory): Additional
+	parameter for pattern filling of memory regions
+	
 2012-09-27  Yao Qi  <yao@codesourcery.com>
 
 	PR breakpoints/13898
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f1d21bc..6f2eba1 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,27 +1665,63 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
+  back_to = make_cleanup (xfree, databuf);
 
   for (i = 0; i < len; ++i)
     {
       int x;
       sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      databuf[i] = (gdb_byte) x;
+    }
+
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len; /* there may be some spare bytes.  */
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0) /* copy spare bytes too.  */
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else if (len > count)
+    {
+      /* Pattern is longer than count: 
+         just copy len bytes.  */
+      data = xmalloc (len);
+      make_cleanup (xfree, data);
+      memcpy (data, databuf, len);
+    }
+  else
+    {
+      /* Pattern is equal to count: 
+         we can just use databuf.  */
+      data = databuf;
     }
 
-  r = target_write_memory (addr, data, len);
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6707751..490b212 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-27-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-09-26  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/dw2-common-block.S: New file.
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..3df6a96
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,68 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+# Copyright (C) 2012 STMicroelectronics
+
+# 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/>.
+
+#
+# test basic Machine interface (MI) operations
+#
+# Verify that, using the MI, we can load a program and do
+# other basic things that are used by all test files through  mi_gdb_exit,
+# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
+# mi_gdb_load, so we can safely use those.
+#
+# The goal is not to test gdb functionality, which is done by other tests,
+# but the command syntax and correct output response to MI operations.
+# 
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if {[build_executable ${testfile}.exp ${binfile} ${srcfile}.c {debug additional_flags=-DFAKEARGV}] == -1} {
+    return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit
-- 
1.7.4


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-09-27 15:27                       ` Giuseppe MONTALTO
@ 2012-09-28 20:07                         ` Tom Tromey
  2012-10-01  9:29                           ` Giuseppe MONTALTO
  2012-10-18 15:41                         ` Pedro Alves
  1 sibling, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-09-28 20:07 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: gdb-patches, Abid, Hafiz

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> the attached patch should fix all the issues.

Thanks.  Unfortunately I thought of one more little thing.

Giuseppe> +      remainder = count % len; /* there may be some spare bytes.  */

Comment should start with a capital letter.
Though TBH you could also just remove this comment since I think the
code is already clear.

Giuseppe> +      if (remainder > 0) /* copy spare bytes too.  */

Likewise on both counts.

Giuseppe> +  else if (len > count)
Giuseppe> +    {
Giuseppe> +      /* Pattern is longer than count: 
Giuseppe> +         just copy len bytes.  */
Giuseppe> +      data = xmalloc (len);
Giuseppe> +      make_cleanup (xfree, data);
Giuseppe> +      memcpy (data, databuf, len);

There's no need to copy the data here.
Just set data=databuf.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-09-28 20:07                         ` Tom Tromey
@ 2012-10-01  9:29                           ` Giuseppe MONTALTO
  2012-10-17 20:59                             ` Tom Tromey
  0 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-10-01  9:29 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Abid, Hafiz

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

New patch attached:
- removed trivial comments
- removed redundant "else if" clause 

Any further feedback is always welcome,
	Giuseppe.

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Friday, September 28, 2012 10:07 PM
> To: Giuseppe MONTALTO
> Cc: gdb-patches@sourceware.org; Abid, Hafiz
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> >>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com>
> writes:
> 
> Giuseppe> the attached patch should fix all the issues.
> 
> Thanks.  Unfortunately I thought of one more little thing.
> 
> Giuseppe> +      remainder = count % len; /* there may be some spare bytes.
> */
> 
> Comment should start with a capital letter.
> Though TBH you could also just remove this comment since I think the
> code is already clear.
> 
> Giuseppe> +      if (remainder > 0) /* copy spare bytes too.  */
> 
> Likewise on both counts.
> 
> Giuseppe> +  else if (len > count)
> Giuseppe> +    {
> Giuseppe> +      /* Pattern is longer than count:
> Giuseppe> +         just copy len bytes.  */
> Giuseppe> +      data = xmalloc (len);
> Giuseppe> +      make_cleanup (xfree, data);
> Giuseppe> +      memcpy (data, databuf, len);
> 
> There's no need to copy the data here.
> Just set data=databuf.
> 
> Tom

[-- Attachment #2: patch-V.7.patch --]
[-- Type: application/octet-stream, Size: 5658 bytes --]

 gdb/ChangeLog                           |    5 ++
 gdb/mi/mi-main.c                        |   45 +++++++++++++++++----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   68 +++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 959e90d..060cb93 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-09-27  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c  (mi_cmd_data_write_memory): Additional
+	parameter for pattern filling of memory regions
+	
 2012-09-27  Yao Qi  <yao@codesourcery.com>
 
 	PR breakpoints/13898
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f1d21bc..b083685 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,27 +1665,55 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
+  back_to = make_cleanup (xfree, databuf);
 
   for (i = 0; i < len; ++i)
     {
       int x;
       sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      databuf[i] = (gdb_byte) x;
+    }
+
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len;
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0)
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else 
+    {
+      /* Pattern is longer than or equal to count: 
+         just copy len bytes.  */
+      data = databuf;
     }
 
-  r = target_write_memory (addr, data, len);
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6707751..490b212 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-27-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-09-26  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/dw2-common-block.S: New file.
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..3df6a96
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,68 @@
+# Copyright (C) 2012 Free Software Foundation, Inc.
+# Copyright (C) 2012 STMicroelectronics
+
+# 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/>.
+
+#
+# test basic Machine interface (MI) operations
+#
+# Verify that, using the MI, we can load a program and do
+# other basic things that are used by all test files through  mi_gdb_exit,
+# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
+# mi_gdb_load, so we can safely use those.
+#
+# The goal is not to test gdb functionality, which is done by other tests,
+# but the command syntax and correct output response to MI operations.
+# 
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if {[build_executable ${testfile}.exp ${binfile} ${srcfile}.c {debug additional_flags=-DFAKEARGV}] == -1} {
+    return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-01  9:29                           ` Giuseppe MONTALTO
@ 2012-10-17 20:59                             ` Tom Tromey
  2012-10-18 13:40                               ` Giuseppe MONTALTO
  0 siblings, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-10-17 20:59 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: gdb-patches, Abid, Hafiz

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Sorry about the delay on this.

Giuseppe> New patch attached:
Giuseppe> - removed trivial comments
Giuseppe> - removed redundant "else if" clause 
Giuseppe> Any further feedback is always welcome,

The patch looks good.
Is there a documentation part as well?  I forget.
If so, and it has been approved, it should all go in with a single
commit.
If not, then you need to patch gdb.texinfo and NEWS as well.

thanks,
Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-17 20:59                             ` Tom Tromey
@ 2012-10-18 13:40                               ` Giuseppe MONTALTO
  0 siblings, 0 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-10-18 13:40 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Abid, Hafiz

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

Thanks for the feedback,

I attached an additional patch, containing the requested changes in NEWS 
and gdb.texinfo.

Please, consider revising it, and forgive me in advance for mistreating the 
English grammar!

Regards,
	Giuseppe

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Wednesday, October 17, 2012 10:59 PM
> To: Giuseppe MONTALTO
> Cc: gdb-patches@sourceware.org; Abid, Hafiz
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> >>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com>
> writes:
> 
> Sorry about the delay on this.
> 
> Giuseppe> New patch attached:
> Giuseppe> - removed trivial comments
> Giuseppe> - removed redundant "else if" clause Any further feedback is
> Giuseppe> always welcome,
> 
> The patch looks good.
> Is there a documentation part as well?  I forget.
> If so, and it has been approved, it should all go in with a single commit.
> If not, then you need to patch gdb.texinfo and NEWS as well.
> 
> thanks,
> Tom

[-- Attachment #2: PATCH_addendum.patch --]
[-- Type: application/octet-stream, Size: 1711 bytes --]

 gdb/NEWS            |    2 ++
 gdb/doc/gdb.texinfo |   11 ++++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index abd0932..f32e51e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -57,6 +57,8 @@ py [command]
      using new async records "=tsv-created" and "=tsv-deleted".
   ** The start and stop of process record are now notified using new
      async record "=record-started" and "=record-stopped".
+  ** New optional parameter added to the "-data-write-memory-bytes" command, 
+     to allow pattern filling of memory areas.
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..13fc63f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31192,7 +31192,7 @@ The corresponding @value{GDBN} command is @samp{x}.
 @subsubheading Synopsis
 
 @smallexample
- -data-write-memory-bytes @var{address} @var{contents}
+ -data-write-memory-bytes @var{address} @var{contents} @r{[}@var{count}@r{]} 
 @end smallexample
 
 @noindent
@@ -31207,6 +31207,9 @@ quoted using the C convention.
 @item @var{contents}
 The hex-encoded bytes to write.
 
+@item @var{count}
+Optional argument indicating the number of bytes to be written.  If @var{count} is bigger than @var{contents}' length, pattern filling occurs.
+
 @end table
 
 @subsubheading @value{GDBN} Command
@@ -31222,6 +31225,12 @@ There's no corresponding @value{GDBN} command.
 (gdb)
 @end smallexample
 
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd" 16e
+^done
+(gdb)
+@end smallexample
 
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Tracepoint Commands

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-09-27 15:27                       ` Giuseppe MONTALTO
  2012-09-28 20:07                         ` Tom Tromey
@ 2012-10-18 15:41                         ` Pedro Alves
  2012-10-18 16:16                           ` Giuseppe MONTALTO
                                             ` (2 more replies)
  1 sibling, 3 replies; 37+ messages in thread
From: Pedro Alves @ 2012-10-18 15:41 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Tom Tromey, gdb-patches, Abid, Hafiz

On 09/27/2012 04:26 PM, Giuseppe MONTALTO wrote:
> +++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> @@ -0,0 +1,68 @@
> +# Copyright (C) 2012 Free Software Foundation, Inc.
> +# Copyright (C) 2012 STMicroelectronics

Sorry, this is not OK.  In order to accept it, the copyright needs to be assigned
to the FSF, only.


While at it:

> +
> +	* mi/mi-main.c  (mi_cmd_data_write_memory): Additional

                      ^^  single space here.


> +	parameter for pattern filling of memory regions
                                                       ^
Missing period.

I'd mention COUNT explicitly, to help grepping.  Thus:

	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
	parameter COUNT, for pattern filling of memory regions.


> +if {[build_executable ${testfile}.exp ${binfile} ${srcfile}.c {debug additional_flags=-DFAKEARGV}] == -1} {

This FAKEARGV usage looks like an unnecessary copy&paste.  Please remove it.
It was removed from mi-read-memory.exp too on 2012-07-10.

> +# test basic Machine interface (MI) operations
> +#
> +# Verify that, using the MI, we can load a program and do
> +# other basic things that are used by all test files through  mi_gdb_exit,
> +# mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and
> +# mi_gdb_load, so we can safely use those.
> +#
> +# The goal is not to test gdb functionality, which is done by other tests,
> +# but the command syntax and correct output response to MI operations.
> +#

All this text too.  I see that it's been blindly copied to a _lot_ of files.. :-/

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-18 15:41                         ` Pedro Alves
@ 2012-10-18 16:16                           ` Giuseppe MONTALTO
  2012-10-18 16:28                             ` [+docs] " Pedro Alves
  2012-10-18 17:30                             ` Eli Zaretskii
  2012-10-18 16:38                           ` Giuseppe MONTALTO
  2012-10-18 19:45                           ` Tom Tromey
  2 siblings, 2 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-10-18 16:16 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches, Abid, Hafiz

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

Thanks again,

Please, find attached a new patch with all the requested changes (also includes
The additons to NEWS and gdb.texinfo that I previously posted in a separate patch).

Regards,
	Giuseppe

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Thursday, October 18, 2012 5:41 PM
> To: Giuseppe MONTALTO
> Cc: Tom Tromey; gdb-patches@sourceware.org; Abid, Hafiz
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> On 09/27/2012 04:26 PM, Giuseppe MONTALTO wrote:
> > +++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> > @@ -0,0 +1,68 @@
> > +# Copyright (C) 2012 Free Software Foundation, Inc.
> > +# Copyright (C) 2012 STMicroelectronics
> 
> Sorry, this is not OK.  In order to accept it, the copyright needs to be assigned
> to the FSF, only.
> 
> 
> While at it:
> 
> > +
> > +	* mi/mi-main.c  (mi_cmd_data_write_memory): Additional
> 
>                       ^^  single space here.
> 
> 
> > +	parameter for pattern filling of memory regions
>                                                        ^ Missing period.
> 
> I'd mention COUNT explicitly, to help grepping.  Thus:
> 
> 	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
> 	parameter COUNT, for pattern filling of memory regions.
> 
> 
> > +if {[build_executable ${testfile}.exp ${binfile} ${srcfile}.c {debug
> > +additional_flags=-DFAKEARGV}] == -1} {
> 
> This FAKEARGV usage looks like an unnecessary copy&paste.  Please remove
> it.
> It was removed from mi-read-memory.exp too on 2012-07-10.
> 
> > +# test basic Machine interface (MI) operations # # Verify that, using
> > +the MI, we can load a program and do # other basic things that are
> > +used by all test files through  mi_gdb_exit, # mi_gdb_start,
> > +mi_delete_breakpoints, mi_gdb_reinitialize_dir and # mi_gdb_load, so
> > +we can safely use those.
> > +#
> > +# The goal is not to test gdb functionality, which is done by other
> > +tests, # but the command syntax and correct output response to MI
> operations.
> > +#
> 
> All this text too.  I see that it's been blindly copied to a _lot_ of files.. :-/
> 
> --
> Pedro Alves


[-- Attachment #2: patch-V.8.patch --]
[-- Type: application/octet-stream, Size: 7065 bytes --]

 gdb/ChangeLog                           |    5 +++
 gdb/NEWS                                |    2 +
 gdb/doc/gdb.texinfo                     |   11 +++++-
 gdb/mi/mi-main.c                        |   45 +++++++++++++++++++----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   58 +++++++++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 959e90d..0bb6806 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-09-27  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
+	parameter COUNT, for pattern filling of memory regions.
+
 2012-09-27  Yao Qi  <yao@codesourcery.com>
 
 	PR breakpoints/13898
diff --git a/gdb/NEWS b/gdb/NEWS
index abd0932..f32e51e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -57,6 +57,8 @@ py [command]
      using new async records "=tsv-created" and "=tsv-deleted".
   ** The start and stop of process record are now notified using new
      async record "=record-started" and "=record-stopped".
+  ** New optional parameter added to the "-data-write-memory-bytes" command, 
+     to allow pattern filling of memory areas.
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..13fc63f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31192,7 +31192,7 @@ The corresponding @value{GDBN} command is @samp{x}.
 @subsubheading Synopsis
 
 @smallexample
- -data-write-memory-bytes @var{address} @var{contents}
+ -data-write-memory-bytes @var{address} @var{contents} @r{[}@var{count}@r{]} 
 @end smallexample
 
 @noindent
@@ -31207,6 +31207,9 @@ quoted using the C convention.
 @item @var{contents}
 The hex-encoded bytes to write.
 
+@item @var{count}
+Optional argument indicating the number of bytes to be written.  If @var{count} is bigger than @var{contents}' length, pattern filling occurs.
+
 @end table
 
 @subsubheading @value{GDBN} Command
@@ -31222,6 +31225,12 @@ There's no corresponding @value{GDBN} command.
 (gdb)
 @end smallexample
 
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd" 16e
+^done
+(gdb)
+@end smallexample
 
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Tracepoint Commands
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f1d21bc..b083685 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,27 +1665,55 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
+  back_to = make_cleanup (xfree, databuf);
 
   for (i = 0; i < len; ++i)
     {
       int x;
       sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      databuf[i] = (gdb_byte) x;
+    }
+
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len;
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0)
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else 
+    {
+      /* Pattern is longer than or equal to count: 
+         just copy len bytes.  */
+      data = databuf;
     }
 
-  r = target_write_memory (addr, data, len);
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6707751..490b212 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-27-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-09-26  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/dw2-common-block.S: New file.
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..344f3fa
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,58 @@
+# Copyright (C) 2012 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/>.
+
+#
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+     untested mi-fill-memory.exp
+     return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit

^ permalink raw reply	[flat|nested] 37+ messages in thread

* [+docs] [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-18 16:16                           ` Giuseppe MONTALTO
@ 2012-10-18 16:28                             ` Pedro Alves
  2012-10-18 17:31                               ` Eli Zaretskii
  2012-10-18 17:30                             ` Eli Zaretskii
  1 sibling, 1 reply; 37+ messages in thread
From: Pedro Alves @ 2012-10-18 16:28 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Tom Tromey, gdb-patches, Abid, Hafiz, Eli Zaretskii

On 10/18/2012 05:16 PM, Giuseppe MONTALTO wrote:
> Thanks again,
> 
> Please, find attached a new patch with all the requested changes (also includes
> The additons to NEWS and gdb.texinfo that I previously posted in a separate patch).

Thanks.  You'll need ChangeLog entries for the NEWS and gdb.texinfo changes as well.
The change to doc/gdb.texinfo is logged in doc/ChangeLog.

Eli [now CCed], our docs maintainer, usually reviews manual changes.  Please wait for
his OK.

Thanks again.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-18 15:41                         ` Pedro Alves
  2012-10-18 16:16                           ` Giuseppe MONTALTO
@ 2012-10-18 16:38                           ` Giuseppe MONTALTO
  2012-10-18 19:45                           ` Tom Tromey
  2 siblings, 0 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-10-18 16:38 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, gdb-patches, Abid, Hafiz

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

I apologize!

Please, discard the previous patch, since it contains a (rather trivial...) error in 
the Expect script.

I attached the correct one (V.9).

Thanks,
	Giuseppe

> -----Original Message-----
> From: Giuseppe MONTALTO
> Sent: Thursday, October 18, 2012 6:17 PM
> To: 'Pedro Alves'
> Cc: Tom Tromey; gdb-patches@sourceware.org; Abid, Hafiz
> Subject: RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> Thanks again,
> 
> Please, find attached a new patch with all the requested changes (also
> includes The additons to NEWS and gdb.texinfo that I previously posted in a
> separate patch).
> 
> Regards,
> 	Giuseppe
> 
> > -----Original Message-----
> > From: Pedro Alves [mailto:palves@redhat.com]
> > Sent: Thursday, October 18, 2012 5:41 PM
> > To: Giuseppe MONTALTO
> > Cc: Tom Tromey; gdb-patches@sourceware.org; Abid, Hafiz
> > Subject: Re: [PATCH] enhancement of
> mi_cmd_data_write_memory_bytes for
> > filling memory regions (was [PATCH] new MI command for pattern filling
> > of memory regions)
> >
> > On 09/27/2012 04:26 PM, Giuseppe MONTALTO wrote:
> > > +++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
> > > @@ -0,0 +1,68 @@
> > > +# Copyright (C) 2012 Free Software Foundation, Inc.
> > > +# Copyright (C) 2012 STMicroelectronics
> >
> > Sorry, this is not OK.  In order to accept it, the copyright needs to
> > be assigned to the FSF, only.
> >
> >
> > While at it:
> >
> > > +
> > > +	* mi/mi-main.c  (mi_cmd_data_write_memory): Additional
> >
> >                       ^^  single space here.
> >
> >
> > > +	parameter for pattern filling of memory regions
> >                                                        ^ Missing period.
> >
> > I'd mention COUNT explicitly, to help grepping.  Thus:
> >
> > 	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
> > 	parameter COUNT, for pattern filling of memory regions.
> >
> >
> > > +if {[build_executable ${testfile}.exp ${binfile} ${srcfile}.c
> > > +{debug additional_flags=-DFAKEARGV}] == -1} {
> >
> > This FAKEARGV usage looks like an unnecessary copy&paste.  Please
> > remove it.
> > It was removed from mi-read-memory.exp too on 2012-07-10.
> >
> > > +# test basic Machine interface (MI) operations # # Verify that,
> > > +using the MI, we can load a program and do # other basic things
> > > +that are used by all test files through  mi_gdb_exit, #
> > > +mi_gdb_start, mi_delete_breakpoints, mi_gdb_reinitialize_dir and #
> > > +mi_gdb_load, so we can safely use those.
> > > +#
> > > +# The goal is not to test gdb functionality, which is done by other
> > > +tests, # but the command syntax and correct output response to MI
> > operations.
> > > +#
> >
> > All this text too.  I see that it's been blindly copied to a _lot_ of
> > files.. :-/
> >
> > --
> > Pedro Alves


[-- Attachment #2: patch-V.9.patch --]
[-- Type: application/octet-stream, Size: 7067 bytes --]

 gdb/ChangeLog                           |    5 +++
 gdb/NEWS                                |    2 +
 gdb/doc/gdb.texinfo                     |   11 +++++-
 gdb/mi/mi-main.c                        |   45 +++++++++++++++++++----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   58 +++++++++++++++++++++++++++++++
 6 files changed, 116 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 959e90d..0bb6806 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2012-09-27  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
+	parameter COUNT, for pattern filling of memory regions.
+
 2012-09-27  Yao Qi  <yao@codesourcery.com>
 
 	PR breakpoints/13898
diff --git a/gdb/NEWS b/gdb/NEWS
index abd0932..f32e51e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -57,6 +57,8 @@ py [command]
      using new async records "=tsv-created" and "=tsv-deleted".
   ** The start and stop of process record are now notified using new
      async record "=record-started" and "=record-stopped".
+  ** New optional parameter added to the "-data-write-memory-bytes" command, 
+     to allow pattern filling of memory areas.
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..13fc63f 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31192,7 +31192,7 @@ The corresponding @value{GDBN} command is @samp{x}.
 @subsubheading Synopsis
 
 @smallexample
- -data-write-memory-bytes @var{address} @var{contents}
+ -data-write-memory-bytes @var{address} @var{contents} @r{[}@var{count}@r{]} 
 @end smallexample
 
 @noindent
@@ -31207,6 +31207,9 @@ quoted using the C convention.
 @item @var{contents}
 The hex-encoded bytes to write.
 
+@item @var{count}
+Optional argument indicating the number of bytes to be written.  If @var{count} is bigger than @var{contents}' length, pattern filling occurs.
+
 @end table
 
 @subsubheading @value{GDBN} Command
@@ -31222,6 +31225,12 @@ There's no corresponding @value{GDBN} command.
 (gdb)
 @end smallexample
 
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd" 16e
+^done
+(gdb)
+@end smallexample
 
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Tracepoint Commands
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f1d21bc..b083685 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,27 +1665,55 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
+  back_to = make_cleanup (xfree, databuf);
 
   for (i = 0; i < len; ++i)
     {
       int x;
       sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      databuf[i] = (gdb_byte) x;
+    }
+
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len;
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0)
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else 
+    {
+      /* Pattern is longer than or equal to count: 
+         just copy len bytes.  */
+      data = databuf;
     }
 
-  r = target_write_memory (addr, data, len);
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6707751..490b212 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-27-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-09-26  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/dw2-common-block.S: New file.
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..ef7004d
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,58 @@
+# Copyright (C) 2012 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/>.
+
+#
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}.c" "${binfile}" executable {debug}] != "" } {
+     untested mi-fill-memory.exp
+     return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-18 16:16                           ` Giuseppe MONTALTO
  2012-10-18 16:28                             ` [+docs] " Pedro Alves
@ 2012-10-18 17:30                             ` Eli Zaretskii
  2012-10-19 14:12                               ` Giuseppe MONTALTO
  1 sibling, 1 reply; 37+ messages in thread
From: Eli Zaretskii @ 2012-10-18 17:30 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: palves, tromey, gdb-patches, Hafiz_Abid

> From: Giuseppe MONTALTO <giuseppe.montalto@st.com>
> Cc: Tom Tromey <tromey@redhat.com>,	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,	"Abid, Hafiz" <Hafiz_Abid@mentor.com>
> Date: Thu, 18 Oct 2012 18:16:34 +0200
> 
> Please, find attached a new patch with all the requested changes (also includes
> The additons to NEWS and gdb.texinfo that I previously posted in a separate patch).

Thanks.

> +  ** New optional parameter added to the "-data-write-memory-bytes" command, 
> +     to allow pattern filling of memory areas.

Please name the new parameter.  Otherwise, this part is OK.

> +@item @var{count}
> +Optional argument indicating the number of bytes to be written.  If @var{count} is bigger than @var{contents}' length, pattern filling occurs.

What exactly does "pattern filling" mean?  This should be explained.
Looking at the code, I think you mean

  If @var{count} is greater than @var{contents}' length, @value{GDBN}
  will repeatedly write @var{contents} until it fills @var{count}
  bytes.

The patch for the manual is OK with these changes.

>    for (i = 0; i < len; ++i)
>      {
>        int x;
>        sscanf (cdata + i * 2, "%02x", &x);
> -      data[i] = (gdb_byte) x;
> +      databuf[i] = (gdb_byte) x;
> +    }

Sorry for chiming in late wrt the code parts, but:

  . you don't test the return value of sscanf, is that a good idea?

  . won't it be better to use strtoul here? that would avoid the need
    to cast to gdb_byte, AFAIU


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [+docs] [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-18 16:28                             ` [+docs] " Pedro Alves
@ 2012-10-18 17:31                               ` Eli Zaretskii
  0 siblings, 0 replies; 37+ messages in thread
From: Eli Zaretskii @ 2012-10-18 17:31 UTC (permalink / raw)
  To: Pedro Alves; +Cc: giuseppe.montalto, tromey, gdb-patches, Hafiz_Abid

> Date: Thu, 18 Oct 2012 17:28:14 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: Tom Tromey <tromey@redhat.com>,
>         "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
>         "Abid, Hafiz" <Hafiz_Abid@mentor.com>, Eli Zaretskii <eliz@gnu.org>
> 
> Thanks.  You'll need ChangeLog entries for the NEWS and gdb.texinfo changes as well.
> The change to doc/gdb.texinfo is logged in doc/ChangeLog.
> 
> Eli [now CCed], our docs maintainer, usually reviews manual changes.  Please wait for
> his OK.

Done (modulo the missing entry for gdb/doc/ChangeLog).


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-18 15:41                         ` Pedro Alves
  2012-10-18 16:16                           ` Giuseppe MONTALTO
  2012-10-18 16:38                           ` Giuseppe MONTALTO
@ 2012-10-18 19:45                           ` Tom Tromey
  2 siblings, 0 replies; 37+ messages in thread
From: Tom Tromey @ 2012-10-18 19:45 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Giuseppe MONTALTO, gdb-patches, Abid, Hafiz

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> On 09/27/2012 04:26 PM, Giuseppe MONTALTO wrote:
>> +++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
>> @@ -0,0 +1,68 @@
>> +# Copyright (C) 2012 Free Software Foundation, Inc.
>> +# Copyright (C) 2012 STMicroelectronics

Pedro> Sorry, this is not OK.  In order to accept it, the copyright
Pedro> needs to be assigned to the FSF, only.

Thanks for catching this.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-18 17:30                             ` Eli Zaretskii
@ 2012-10-19 14:12                               ` Giuseppe MONTALTO
  2012-10-19 16:34                                 ` Eli Zaretskii
  2012-10-19 16:56                                 ` Pedro Alves
  0 siblings, 2 replies; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-10-19 14:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, tromey, gdb-patches, Hafiz_Abid

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

Dear all,
I've performed all the required changes.

Incidentally, the attached patch contains the sscanf() version of the loop mentioned here below:

  for (i = 0; i < len; ++i)
    {
      int x;
      if (sscanf (cdata + i * 2, "%02x", &x) != 1)
        error (_("Invalid argument"));
      databuf[i] = (gdb_byte) x;
    }

While, on the other hand, using strtoul() instead of sscanf() will lead to something 
like this, or so I guess:

  for (i = 0; i < len; ++i)
      databuf[i] = strtoul (cdata + i * 2, NULL, 16);

The former still being my preferred one, I think it all comes down to personal taste, doesn't it?

Regards,
	Giuseppe

> -----Original Message-----
> From: Eli Zaretskii [mailto:eliz@gnu.org]
> Sent: Thursday, October 18, 2012 7:31 PM
> To: Giuseppe MONTALTO
> Cc: palves@redhat.com; tromey@redhat.com; gdb-
> patches@sourceware.org; Hafiz_Abid@mentor.com
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> > From: Giuseppe MONTALTO <giuseppe.montalto@st.com>
> > Cc: Tom Tromey <tromey@redhat.com>,	"gdb-
> patches@sourceware.org" <gdb-patches@sourceware.org>,	"Abid, Hafiz"
> <Hafiz_Abid@mentor.com>
> > Date: Thu, 18 Oct 2012 18:16:34 +0200
> >
> > Please, find attached a new patch with all the requested changes (also
> > includes The additons to NEWS and gdb.texinfo that I previously posted in a
> separate patch).
> 
> Thanks.
> 
> > +  ** New optional parameter added to the "-data-write-memory-bytes"
> command,
> > +     to allow pattern filling of memory areas.
> 
> Please name the new parameter.  Otherwise, this part is OK.
> 
> > +@item @var{count}
> > +Optional argument indicating the number of bytes to be written.  If
> @var{count} is bigger than @var{contents}' length, pattern filling occurs.
> 
> What exactly does "pattern filling" mean?  This should be explained.
> Looking at the code, I think you mean
> 
>   If @var{count} is greater than @var{contents}' length, @value{GDBN}
>   will repeatedly write @var{contents} until it fills @var{count}
>   bytes.
> 
> The patch for the manual is OK with these changes.
> 
> >    for (i = 0; i < len; ++i)
> >      {
> >        int x;
> >        sscanf (cdata + i * 2, "%02x", &x);
> > -      data[i] = (gdb_byte) x;
> > +      databuf[i] = (gdb_byte) x;
> > +    }
> 
> Sorry for chiming in late wrt the code parts, but:
> 
>   . you don't test the return value of sscanf, is that a good idea?
> 
>   . won't it be better to use strtoul here? that would avoid the need
>     to cast to gdb_byte, AFAIU


[-- Attachment #2: patch-V.10.patch --]
[-- Type: application/octet-stream, Size: 7798 bytes --]

 gdb/ChangeLog                           |    6 +++
 gdb/NEWS                                |    2 +
 gdb/doc/ChangeLog                       |    5 +++
 gdb/doc/gdb.texinfo                     |   13 ++++++-
 gdb/mi/mi-main.c                        |   48 +++++++++++++++++++++-----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   58 +++++++++++++++++++++++++++++++
 7 files changed, 126 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 959e90d..e88d1b8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2012-10-19  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
+	parameter COUNT, for pattern filling of memory regions.
+	* NEWS: Mention it.
+
 2012-09-27  Yao Qi  <yao@codesourcery.com>
 
 	PR breakpoints/13898
diff --git a/gdb/NEWS b/gdb/NEWS
index abd0932..a29fa15 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -57,6 +57,8 @@ py [command]
      using new async records "=tsv-created" and "=tsv-deleted".
   ** The start and stop of process record are now notified using new
      async record "=record-started" and "=record-stopped".
+  ** New optional parameter COUNT added to the "-data-write-memory-bytes"  
+     command, to allow pattern filling of memory areas.
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 05ac406..aae41fc 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2012-10-19  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.texinfo (-data-write-memory-bytes): Changed the command
+	description and added an example to reflect current implementation.
+
 2012-09-26  Siddhesh Poyarekar  <siddhesh@redhat.com>
 
 	* observer.texi (memory_changed): Expand parameter LEN to ssize_t.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..868e7c0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31192,7 +31192,7 @@ The corresponding @value{GDBN} command is @samp{x}.
 @subsubheading Synopsis
 
 @smallexample
- -data-write-memory-bytes @var{address} @var{contents}
+ -data-write-memory-bytes @var{address} @var{contents} @r{[}@var{count}@r{]} 
 @end smallexample
 
 @noindent
@@ -31207,6 +31207,11 @@ quoted using the C convention.
 @item @var{contents}
 The hex-encoded bytes to write.
 
+@item @var{count}
+Optional argument indicating the number of bytes to be written.  If @var{count} 
+is greater than @var{contents}' length, @value{GDBN} will repeatedly 
+write @var{contents} until it fills @var{count} bytes.
+
 @end table
 
 @subsubheading @value{GDBN} Command
@@ -31222,6 +31227,12 @@ There's no corresponding @value{GDBN} command.
 (gdb)
 @end smallexample
 
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd" 16e
+^done
+(gdb)
+@end smallexample
 
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Tracepoint Commands
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f1d21bc..50816f5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,27 +1665,56 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
+  back_to = make_cleanup (xfree, databuf);
 
   for (i = 0; i < len; ++i)
     {
       int x;
-      sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      if (sscanf (cdata + i * 2, "%02x", &x) != 1)
+        error (_("Invalid argument"));
+      databuf[i] = (gdb_byte) x;
+    }
+
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len;
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0)
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else 
+    {
+      /* Pattern is longer than or equal to count: 
+         just copy len bytes.  */
+      data = databuf;
     }
 
-  r = target_write_memory (addr, data, len);
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6707751..490b212 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-27-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-09-26  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/dw2-common-block.S: New file.
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..ef7004d
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,58 @@
+# Copyright (C) 2012 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/>.
+
+#
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}.c" "${binfile}" executable {debug}] != "" } {
+     untested mi-fill-memory.exp
+     return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-19 14:12                               ` Giuseppe MONTALTO
@ 2012-10-19 16:34                                 ` Eli Zaretskii
  2012-10-19 16:56                                 ` Pedro Alves
  1 sibling, 0 replies; 37+ messages in thread
From: Eli Zaretskii @ 2012-10-19 16:34 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: palves, tromey, gdb-patches, Hafiz_Abid

> From: Giuseppe MONTALTO <giuseppe.montalto@st.com>
> Cc: "palves@redhat.com" <palves@redhat.com>,
> 	"tromey@redhat.com" <tromey@redhat.com>,
> 	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
> 	"Hafiz_Abid@mentor.com" <Hafiz_Abid@mentor.com>
> Date: Fri, 19 Oct 2012 16:11:50 +0200
> 
> Incidentally, the attached patch contains the sscanf() version of the loop mentioned here below:
> 
>   for (i = 0; i < len; ++i)
>     {
>       int x;
>       if (sscanf (cdata + i * 2, "%02x", &x) != 1)
>         error (_("Invalid argument"));
>       databuf[i] = (gdb_byte) x;
>     }
> 
> While, on the other hand, using strtoul() instead of sscanf() will lead to something 
> like this, or so I guess:
> 
>   for (i = 0; i < len; ++i)
>       databuf[i] = strtoul (cdata + i * 2, NULL, 16);
> 
> The former still being my preferred one, I think it all comes down to personal taste, doesn't it?

Not necessarily.  If you pass a non-NULL 2nd argument to strtoul, you
can check for errors with better accuracy.


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-19 14:12                               ` Giuseppe MONTALTO
  2012-10-19 16:34                                 ` Eli Zaretskii
@ 2012-10-19 16:56                                 ` Pedro Alves
  2012-10-22  7:02                                   ` Giuseppe MONTALTO
  1 sibling, 1 reply; 37+ messages in thread
From: Pedro Alves @ 2012-10-19 16:56 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Eli Zaretskii, tromey, gdb-patches, Hafiz_Abid

On 10/19/2012 03:11 PM, Giuseppe MONTALTO wrote:
> +	* gdb.texinfo (-data-write-memory-bytes): Changed the command
> +	description and added an example to reflect current implementation.

Don't use past tense:

 s/Changed/Change/
 s/added/add/

The norm is to use the section name as context.  As with the other log entry, I'd
mention exactly what is being changed to help archaeology/grepping.  Thus I suggest:

	* gdb.texinfo (GDB/MI Data Manipulation): Document new optional parameter
	"count" of -data-write-memory-bytes, and add an example.

Thanks.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-19 16:56                                 ` Pedro Alves
@ 2012-10-22  7:02                                   ` Giuseppe MONTALTO
  2012-11-08 20:37                                     ` Tom Tromey
  0 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-10-22  7:02 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Eli Zaretskii, tromey, gdb-patches, Hafiz_Abid

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

Done,

new patch attached.

Thanks,
	Giuseppe

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, October 19, 2012 6:56 PM
> To: Giuseppe MONTALTO
> Cc: Eli Zaretskii; tromey@redhat.com; gdb-patches@sourceware.org;
> Hafiz_Abid@mentor.com
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> On 10/19/2012 03:11 PM, Giuseppe MONTALTO wrote:
> > +	* gdb.texinfo (-data-write-memory-bytes): Changed the command
> > +	description and added an example to reflect current implementation.
> 
> Don't use past tense:
> 
>  s/Changed/Change/
>  s/added/add/
> 
> The norm is to use the section name as context.  As with the other log entry,
> I'd mention exactly what is being changed to help archaeology/grepping.
> Thus I suggest:
> 
> 	* gdb.texinfo (GDB/MI Data Manipulation): Document new optional
> parameter
> 	"count" of -data-write-memory-bytes, and add an example.
> 
> Thanks.
> 
> --
> Pedro Alves


[-- Attachment #2: patch-V.11.patch --]
[-- Type: application/octet-stream, Size: 7800 bytes --]

 gdb/ChangeLog                           |    6 +++
 gdb/NEWS                                |    2 +
 gdb/doc/ChangeLog                       |    5 +++
 gdb/doc/gdb.texinfo                     |   13 ++++++-
 gdb/mi/mi-main.c                        |   48 +++++++++++++++++++++-----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   58 +++++++++++++++++++++++++++++++
 7 files changed, 126 insertions(+), 10 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 959e90d..e88d1b8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2012-10-19  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
+	parameter COUNT, for pattern filling of memory regions.
+	* NEWS: Mention it.
+
 2012-09-27  Yao Qi  <yao@codesourcery.com>
 
 	PR breakpoints/13898
diff --git a/gdb/NEWS b/gdb/NEWS
index abd0932..a29fa15 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -57,6 +57,8 @@ py [command]
      using new async records "=tsv-created" and "=tsv-deleted".
   ** The start and stop of process record are now notified using new
      async record "=record-started" and "=record-stopped".
+  ** New optional parameter COUNT added to the "-data-write-memory-bytes"  
+     command, to allow pattern filling of memory areas.
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 05ac406..552e048 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2012-10-19  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.texinfo (GDB/MI Data Manipulation): Document new optional 
+	parameter "count" of -data-write-memory-bytes, and add an example.
+
 2012-09-26  Siddhesh Poyarekar  <siddhesh@redhat.com>
 
 	* observer.texi (memory_changed): Expand parameter LEN to ssize_t.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 5fcbada..868e7c0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31192,7 +31192,7 @@ The corresponding @value{GDBN} command is @samp{x}.
 @subsubheading Synopsis
 
 @smallexample
- -data-write-memory-bytes @var{address} @var{contents}
+ -data-write-memory-bytes @var{address} @var{contents} @r{[}@var{count}@r{]} 
 @end smallexample
 
 @noindent
@@ -31207,6 +31207,11 @@ quoted using the C convention.
 @item @var{contents}
 The hex-encoded bytes to write.
 
+@item @var{count}
+Optional argument indicating the number of bytes to be written.  If @var{count} 
+is greater than @var{contents}' length, @value{GDBN} will repeatedly 
+write @var{contents} until it fills @var{count} bytes.
+
 @end table
 
 @subsubheading @value{GDBN} Command
@@ -31222,6 +31227,12 @@ There's no corresponding @value{GDBN} command.
 (gdb)
 @end smallexample
 
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd" 16e
+^done
+(gdb)
+@end smallexample
 
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Tracepoint Commands
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index f1d21bc..50816f5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,27 +1665,56 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
+  back_to = make_cleanup (xfree, databuf);
 
   for (i = 0; i < len; ++i)
     {
       int x;
-      sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      if (sscanf (cdata + i * 2, "%02x", &x) != 1)
+        error (_("Invalid argument"));
+      databuf[i] = (gdb_byte) x;
+    }
+
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len;
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0)
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else 
+    {
+      /* Pattern is longer than or equal to count: 
+         just copy len bytes.  */
+      data = databuf;
     }
 
-  r = target_write_memory (addr, data, len);
+  r = target_write_memory (addr, data, count);
   if (r != 0)
     error (_("Could not write memory"));
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 6707751..490b212 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-27-18  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-09-26  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.dwarf2/dw2-common-block.S: New file.
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..ef7004d
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,58 @@
+# Copyright (C) 2012 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/>.
+
+#
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}.c" "${binfile}" executable {debug}] != "" } {
+     untested mi-fill-memory.exp
+     return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-10-22  7:02                                   ` Giuseppe MONTALTO
@ 2012-11-08 20:37                                     ` Tom Tromey
  2012-11-09  8:59                                       ` Giuseppe MONTALTO
  0 siblings, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-11-08 20:37 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Hafiz_Abid

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> new patch attached.

This looks good.  Please commit.

If you are going to write more patches, contact me off-list and I will
get you set up with write-after-approval access.

Otherwise, let me know and I will check it in for you.

BTW, somehow your patch got ^Ms at the end of each line.

thanks,
Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-11-08 20:37                                     ` Tom Tromey
@ 2012-11-09  8:59                                       ` Giuseppe MONTALTO
  2012-11-12 16:52                                         ` Tom Tromey
  0 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-11-09  8:59 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Hafiz_Abid

Thanks,

I'm not going to submit other patches for the moment, so please check it in.

Incidentally, I will submit a RFC on some Expect scripts I'm writing (for c++
validation), but it's a still work in progress, I think they're quite far from being
ready for integration.

Sorry for the ^Ms, probably I edited the patch on Windows before actually
sending it.

Thanks,
	Giuseppe

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Thursday, November 08, 2012 9:37 PM
> To: Giuseppe MONTALTO
> Cc: Pedro Alves; Eli Zaretskii; gdb-patches@sourceware.org;
> Hafiz_Abid@mentor.com
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> >>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com>
> writes:
> 
> Giuseppe> new patch attached.
> 
> This looks good.  Please commit.
> 
> If you are going to write more patches, contact me off-list and I will get you
> set up with write-after-approval access.
> 
> Otherwise, let me know and I will check it in for you.
> 
> BTW, somehow your patch got ^Ms at the end of each line.
> 
> thanks,
> Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-11-09  8:59                                       ` Giuseppe MONTALTO
@ 2012-11-12 16:52                                         ` Tom Tromey
  2012-11-13 10:50                                           ` Giuseppe MONTALTO
  0 siblings, 1 reply; 37+ messages in thread
From: Tom Tromey @ 2012-11-12 16:52 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Hafiz_Abid

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> I'm not going to submit other patches for the moment, so
Giuseppe> please check it in.

The patch didn't apply cleanly to CVS head.  mi-main.c has changed.
Can you please send an updated one?

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

* RE: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-11-12 16:52                                         ` Tom Tromey
@ 2012-11-13 10:50                                           ` Giuseppe MONTALTO
  2012-11-13 21:19                                             ` Tom Tromey
  0 siblings, 1 reply; 37+ messages in thread
From: Giuseppe MONTALTO @ 2012-11-13 10:50 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Hafiz_Abid

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

Updated.
The attached one should fit the CVS head.

Thanks,
	Giuseppe

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com]
> Sent: Monday, November 12, 2012 5:52 PM
> To: Giuseppe MONTALTO
> Cc: Pedro Alves; Eli Zaretskii; gdb-patches@sourceware.org;
> Hafiz_Abid@mentor.com
> Subject: Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes
> for filling memory regions (was [PATCH] new MI command for pattern filling
> of memory regions)
> 
> >>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com>
> writes:
> 
> Giuseppe> I'm not going to submit other patches for the moment, so
> Giuseppe> please check it in.
> 
> The patch didn't apply cleanly to CVS head.  mi-main.c has changed.
> Can you please send an updated one?
> 
> Tom

[-- Attachment #2: patch-V.12.patch --]
[-- Type: application/octet-stream, Size: 7697 bytes --]

28b9deb203c0180fe668ac6434f2ca0e8a56d3ee
 gdb/ChangeLog                           |    6 +++
 gdb/NEWS                                |    2 +
 gdb/doc/ChangeLog                       |    5 +++
 gdb/doc/gdb.texinfo                     |   12 ++++++
 gdb/mi/mi-main.c                        |   48 +++++++++++++++++++++-----
 gdb/testsuite/ChangeLog                 |    4 ++
 gdb/testsuite/gdb.mi/mi-fill-memory.exp |   58 +++++++++++++++++++++++++++++++
 7 files changed, 126 insertions(+), 9 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 250a255..4d7a624 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2012-11-13  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* mi/mi-main.c (mi_cmd_data_write_memory): Handle additional
+	parameter COUNT, for pattern filling of memory regions.
+	* NEWS: Mention it.
+
 2012-11-12  Joel Brobecker  <brobecker@adacore.com>
 
 	* frame.h (deprecated_frame_register_read): Renames
diff --git a/gdb/NEWS b/gdb/NEWS
index 739a7b3..9375218 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -90,6 +90,8 @@ show print type typedefs
   ** The data-disassemble command response will include a "fullname" field
      containing the absolute file name when GDB can determine it and source
      has been requested.
+  ** New optional parameter COUNT added to the "-data-write-memory-bytes" 
+     command, to allow pattern filling of memory areas.
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 0410dbc..65d464a 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2012-11-13  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.texinfo (GDB/MI Data Manipulation): Document new optional 
+	parameter "count" of -data-write-memory-bytes, and add an example.
+
 2012-11-12  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.texinfo (Symbols): Document "info type-printers",
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 86cfe8e..f45b65e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -31443,6 +31443,7 @@ The corresponding @value{GDBN} command is @samp{x}.
 
 @smallexample
  -data-write-memory-bytes @var{address} @var{contents}
+ -data-write-memory-bytes @var{address} @var{contents} @r{[}@var{count}@r{]}
 @end smallexample
 
 @noindent
@@ -31457,6 +31458,11 @@ quoted using the C convention.
 @item @var{contents}
 The hex-encoded bytes to write.
 
+@item @var{count}
+Optional argument indicating the number of bytes to be written.  If @var{count} 
+is greater than @var{contents}' length, @value{GDBN} will repeatedly 
+write @var{contents} until it fills @var{count} bytes.
+
 @end table
 
 @subsubheading @value{GDBN} Command
@@ -31472,6 +31478,12 @@ There's no corresponding @value{GDBN} command.
 (gdb)
 @end smallexample
 
+@smallexample
+(gdb)
+-data-write-memory-bytes &a "aabbccdd" 16e
+^done
+(gdb)
+@end smallexample
 
 @c %%%%%%%%%%%%%%%%%%%%%%%%%%%% SECTION %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 @node GDB/MI Tracepoint Commands
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 9fa1eaa..1b7d68a 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1656,7 +1656,8 @@ mi_cmd_data_write_memory (char *command, char **argv, int argc)
 /* Implementation of the -data-write-memory-bytes command.
 
    ADDR: start address
-   DATA: string of bytes to write at that address.  */
+   DATA: string of bytes to write at that address
+   COUNT: number of bytes to be filled (decimal integer).  */
 
 void
 mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
@@ -1664,11 +1665,13 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
   CORE_ADDR addr;
   char *cdata;
   gdb_byte *data;
-  int len, r, i;
+  gdb_byte *databuf;
+  size_t len, r, i, steps, remainder;
+  long int count, j;
   struct cleanup *back_to;
 
-  if (argc != 2)
-    error (_("Usage: ADDR DATA."));
+  if (argc != 2 && argc != 3)
+    error (_("Usage: ADDR DATA [COUNT]."));
 
   addr = parse_and_eval_address (argv[0]);
   cdata = argv[1];
@@ -1677,18 +1680,45 @@ mi_cmd_data_write_memory_bytes (char *command, char **argv, int argc)
 	   cdata);
 
   len = strlen (cdata)/2;
+  if (argc == 3)
+    count = strtoul (argv[2], NULL, 10);
+  else
+    count = len;
 
-  data = xmalloc (len);
-  back_to = make_cleanup (xfree, data);
+  databuf = xmalloc (len * sizeof (gdb_byte));
+  back_to = make_cleanup (xfree, databuf);
 
   for (i = 0; i < len; ++i)
     {
       int x;
-      sscanf (cdata + i * 2, "%02x", &x);
-      data[i] = (gdb_byte) x;
+      if (sscanf (cdata + i * 2, "%02x", &x) != 1)
+        error (_("Invalid argument"));
+      databuf[i] = (gdb_byte) x;
+    }
+
+  if (len < count)
+    {
+      /* Pattern is made of less bytes than count: 
+         repeat pattern to fill memory.  */
+      data = xmalloc (count);
+      make_cleanup (xfree, data);
+    
+      steps = count / len;
+      remainder = count % len;
+      for (j = 0; j < steps; j++)
+        memcpy (data + j * len, databuf, len);
+
+      if (remainder > 0)
+        memcpy (data + steps * len, databuf, remainder);
+    }
+  else 
+    {
+      /* Pattern is longer than or equal to count: 
+         just copy len bytes.  */
+      data = databuf;
     }
 
-  write_memory_with_notification (addr, data, len);
+  write_memory_with_notification (addr, data, count);
 
   do_cleanups (back_to);
 }
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 5293c88..289f115 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2012-11-13  Giuseppe Montalto  <giuseppe.montalto@st.com>
+
+	* gdb.mi/mi-fill-memory.exp: New test.
+
 2012-11-12  Tom Tromey  <tromey@redhat.com>
 
 	* gdb.base/completion.exp: Update for "info type-printers".
diff --git a/gdb/testsuite/gdb.mi/mi-fill-memory.exp b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
new file mode 100644
index 0000000..ef7004d
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-fill-memory.exp
@@ -0,0 +1,58 @@
+# Copyright (C) 2012 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/>.
+
+#
+# added for testing the -data-write-memory-bytes MI command enhancements
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile "mi-read-memory"
+ 
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}.c" "${binfile}" executable {debug}] != "" } {
+     untested mi-fill-memory.exp
+     return -1
+}
+
+mi_run_to_main
+mi_next_to "main" "" "mi-read-memory.c" "20" "next at main"
+
+mi_gdb_test "1-data-write-memory-bytes"\
+	"1\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"no arguments"
+
+mi_gdb_test "2-data-write-memory-bytes 8"\
+	"2\\\^error,msg=\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\
+	"one argument missing"
+
+mi_gdb_test "3-data-write-memory-bytes \$pc ab"\
+	"3\\\^done" \
+	"memory successfully written"
+
+mi_gdb_test "4-data-write-memory-bytes \$pc ab 8"\
+	"4\\\^done" \
+	"memory successfully filled (8 bytes)"
+
+mi_gdb_test "5-interpreter-exec console \"x \$pc\"" \
+    ".*0xabababab.*" \
+    "pattern correctly read from memory"
+
+mi_gdb_exit

^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions)
  2012-11-13 10:50                                           ` Giuseppe MONTALTO
@ 2012-11-13 21:19                                             ` Tom Tromey
  0 siblings, 0 replies; 37+ messages in thread
From: Tom Tromey @ 2012-11-13 21:19 UTC (permalink / raw)
  To: Giuseppe MONTALTO; +Cc: Pedro Alves, Eli Zaretskii, gdb-patches, Hafiz_Abid

>>>>> "Giuseppe" == Giuseppe MONTALTO <giuseppe.montalto@st.com> writes:

Giuseppe> The attached one should fit the CVS head.

Thank you.  I have checked it in.

Tom


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2012-11-13 21:19 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-09 16:18 [PATCH] new MI command for pattern filling of memory regions Giuseppe MONTALTO
2012-05-09 16:30 ` Tom Tromey
2012-05-09 18:11 ` Tom Tromey
2012-05-10 11:41   ` Giuseppe MONTALTO
2012-05-10 13:57     ` Tom Tromey
2012-05-11  8:53       ` Giuseppe MONTALTO
2012-05-11 14:36         ` Tom Tromey
2012-05-11 16:06           ` Giuseppe MONTALTO
2012-05-25 14:35           ` Giuseppe MONTALTO
2012-06-12  9:07           ` Giuseppe MONTALTO
2012-06-12 10:01             ` Abid, Hafiz
2012-06-12 14:13               ` Giuseppe MONTALTO
2012-06-12 14:22               ` [PATCH] enhancement of mi_cmd_data_write_memory_bytes for filling memory regions (was [PATCH] new MI command for pattern filling of memory regions) Giuseppe MONTALTO
2012-09-14 16:30                 ` Tom Tromey
2012-09-18 15:19                   ` Giuseppe MONTALTO
2012-09-26 20:44                     ` Tom Tromey
2012-09-27 15:27                       ` Giuseppe MONTALTO
2012-09-28 20:07                         ` Tom Tromey
2012-10-01  9:29                           ` Giuseppe MONTALTO
2012-10-17 20:59                             ` Tom Tromey
2012-10-18 13:40                               ` Giuseppe MONTALTO
2012-10-18 15:41                         ` Pedro Alves
2012-10-18 16:16                           ` Giuseppe MONTALTO
2012-10-18 16:28                             ` [+docs] " Pedro Alves
2012-10-18 17:31                               ` Eli Zaretskii
2012-10-18 17:30                             ` Eli Zaretskii
2012-10-19 14:12                               ` Giuseppe MONTALTO
2012-10-19 16:34                                 ` Eli Zaretskii
2012-10-19 16:56                                 ` Pedro Alves
2012-10-22  7:02                                   ` Giuseppe MONTALTO
2012-11-08 20:37                                     ` Tom Tromey
2012-11-09  8:59                                       ` Giuseppe MONTALTO
2012-11-12 16:52                                         ` Tom Tromey
2012-11-13 10:50                                           ` Giuseppe MONTALTO
2012-11-13 21:19                                             ` Tom Tromey
2012-10-18 16:38                           ` Giuseppe MONTALTO
2012-10-18 19:45                           ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox