Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Michael Snyder <msnyder@vmware.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
	Vladimir Prus <vladimir@codesourcery.com>,
	"dan@codesourcery.com" <dan@codesourcery.com>
Subject: Re: [RFA] add test for memattr, use get_number_or_range for memattr commands
Date: Mon, 21 Feb 2011 09:24:00 -0000	[thread overview]
Message-ID: <20110221091304.GB2600@adacore.com> (raw)
In-Reply-To: <4D605267.8030708@vmware.com>

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

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

The code changes look fine to me.

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

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

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


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

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

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

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

-- 
Joel


  reply	other threads:[~2011-02-21  9:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-20  1:44 Michael Snyder
2011-02-21  9:24 ` Joel Brobecker [this message]
2011-02-21 15:06   ` Tom Tromey
2011-02-21 19:55     ` Michael Snyder
2011-02-21 23:40     ` Michael Snyder
2011-02-22  8:51       ` Joel Brobecker
2011-02-22 17:58         ` Michael Snyder
2011-02-23  4:01           ` Joel Brobecker
2011-02-23 18:34             ` Michael Snyder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110221091304.GB2600@adacore.com \
    --to=brobecker@adacore.com \
    --cc=dan@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=msnyder@vmware.com \
    --cc=vladimir@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox