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 MONTALTO
2012-10-01T09:29:39Z

[-- 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