From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13546 invoked by alias); 12 Jun 2012 10:01:12 -0000 Received: (qmail 13538 invoked by uid 22791); 12 Jun 2012 10:01:09 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_CP X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 12 Jun 2012 10:00:43 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1SeNty-0002r7-Aa from Hafiz_Abid@mentor.com ; Tue, 12 Jun 2012 03:00:42 -0700 Received: from SVR-IES-FEM-04.mgc.mentorg.com ([137.202.0.110]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Tue, 12 Jun 2012 03:00:41 -0700 Received: from EU-MBX-03.mgc.mentorg.com ([169.254.2.57]) by SVR-IES-FEM-04.mgc.mentorg.com ([137.202.0.110]) with mapi id 14.01.0289.001; Tue, 12 Jun 2012 11:00:40 +0100 From: "Abid, Hafiz" To: Giuseppe MONTALTO , Tom Tromey , "gdb-patches@sourceware.org" Subject: RE: [PATCH] new MI command for pattern filling of memory regions Date: Tue, 12 Jun 2012 10:01:00 -0000 Message-ID: References: <76FE3225DF13124EA2D05B290B624C95E65F2D466E@SAFEX1MAIL1.st.com> <87sjf9rzr0.fsf@fleche.redhat.com> <76FE3225DF13124EA2D05B290B624C95E65FECE356@SAFEX1MAIL1.st.com> <871umsqgu8.fsf@fleche.redhat.com> <76FE3225DF13124EA2D05B290B624C95E65FECE48F@SAFEX1MAIL1.st.com> <87397624b9.fsf@fleche.redhat.com> <76FE3225DF13124EA2D05B290B624C95E662A6921F@SAFEX1MAIL1.st.com> In-Reply-To: <76FE3225DF13124EA2D05B290B624C95E662A6921F@SAFEX1MAIL1.st.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-06/txt/msg00328.txt.bz2 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 we= eks; is there any chance=20 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 >=20 > I reworked the patch, including the test script; > Accordingly with your feedback, the sscanf cycle is now performed only > once. >=20 > Here's the new patch, please let me know if there is anything more to > change. >=20 > Regards, > Giuseppe >=20 > 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(-) >=20 > 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 > + > + * mi/mi-main.c (mi_cmd_data_write_memory): additional > + parameter for pattern filling of memory regions > + > 2012-05-09 Pedro Alves >=20 > * 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. >=20 > 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). */ >=20 > 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; >=20 > - if (argc !=3D 2) > - error (_("Usage: ADDR DATA.")); > + if (argc !=3D 2 && argc !=3D 3) > + error (_("Usage: ADDR DATA [COUNT].")); >=20 > addr =3D parse_and_eval_address (argv[0]); > cdata =3D argv[1]; > - len =3D strlen (cdata)/2; > + len =3D strlen (cdata) / 2; > + if (argc =3D=3D 3) > + count =3D strtol(argv[2], NULL, 10); > + else > + count =3D len; >=20 > - data =3D xmalloc (len); > - back_to =3D make_cleanup (xfree, data); > + databuf =3D xmalloc (len * sizeof(gdb_byte)); >=20 > for (i =3D 0; i < len; ++i) > { > int x; > sscanf (cdata + i * 2, "%02x", &x); > - data[i] =3D (gdb_byte) x; > + databuf[i] =3D (gdb_byte)x; > } >=20 > - r =3D target_write_memory (addr, data, len); > + if (len < count) > + { > + /* pattern is made of less bytes than count: > + repeat pattern to fill memory. */ > + data =3D xmalloc (count); > + back_to =3D make_cleanup (xfree, data); > + > + steps =3D count / len; > + reminder =3D count % len; /* there may be some spare bytes. */ > + for (j =3D 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 =3D xmalloc (len); > + back_to =3D make_cleanup (xfree, data); > + memcpy (data, databuf, (size_t) len); > + } > + > + xfree(databuf); Please add a space before the opening parenthesis.=20 > + > + r =3D target_write_memory (addr, data, count); > if (r !=3D 0) > error (_("Could not write memory")); >=20 > 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 > + > + * gdb.mi/mi-fill-memory.exp: New test. > + > 2012-05-08 Maciej W. Rozycki >=20 > * 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 > . > + > +# > +# 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=3Dmi" > + > +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=3D-DFAKEARGV}] !=3D "" } { > + 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=3D\"Usage: ADDR DATA \\\[COUNT\\\]\.\""\ > + "no arguments" > + > +mi_gdb_test "2-data-write-memory-bytes 8"\ > + "2\\\^error,msg=3D\"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 >=20 >=20 >=20 > > -----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 =3D xmalloc (count/len); > > > > > > This allocates count/len bytes... > > > > > > Giuseppe> + steps =3D count/len; > > > Giuseppe> + for (j =3D 0; j < steps; j++) > > > Giuseppe> + { > > > Giuseppe> + for (i =3D 0; i < len; ++i) > > > Giuseppe> + { > > > Giuseppe> + int x; > > > Giuseppe> + sscanf (cdata + i * 2, "%02x", &x); > > > Giuseppe> + data[i + j * len] =3D (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