Re: [PATCH 1/3] reggroups: Add test and docs for `info reg $reggroup` feature

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-06-08 00:15, Stafford Horne wrote:
> Until now this feature has existed but was not documented.  Adding docs
> and tests.

Hi Stafford,

Thanks for doing this!  It would be cool if you could improve the 
documentation printed by "help info registers", while at it.

> gdb/ChangeLog:
> 
> 2017-06-06  Stafford Horne  <shorne@gmail.com>
> 
> 	* doc/gdb.texinfo: Document info reg $reggroup feature.
> 
> gdb/testsuite/ChangeLog:
> 
> 2017-06-06  Stafford Horne  <shorne@gmail.com>
> 
> 	* gdb.base/reggroups.c: New file.
> 	* gdb.base/reggroups.exp: New file.
> ---
>  gdb/doc/gdb.texinfo                  |  5 +++
>  gdb/testsuite/gdb.base/reggroups.c   |  4 ++
>  gdb/testsuite/gdb.base/reggroups.exp | 71 
> ++++++++++++++++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/reggroups.c
>  create mode 100644 gdb/testsuite/gdb.base/reggroups.exp
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 9fb70f6..a11db0c 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -10878,6 +10878,11 @@ and vector registers (in the selected stack 
> frame).
>  Print the names and values of all registers, including floating-point
>  and vector registers (in the selected stack frame).
> 
> +@item info registers @var{reggroup} @dots{}
> +Print the name and value of the restiers in each of the specified

Do you mean "registers"?

> +@var{reggroup}.  The @var{reggoup} can be any of those returned by
> +@code{maint print reggroups}.
> +
>  @item info registers @var{regname} @dots{}
>  Print the @dfn{relativized} value of each specified register 
> @var{regname}.
>  As discussed in detail below, register values are normally relative to
> diff --git a/gdb/testsuite/gdb.base/reggroups.c
> b/gdb/testsuite/gdb.base/reggroups.c
> new file mode 100644
> index 0000000..f8b643a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/reggroups.c
> @@ -0,0 +1,4 @@
> +int main()

We try to use the GDB formatting style for the test code as well, so 
here it would be:

int
main ()
{
   return 0;
}

> +{
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/reggroups.exp
> b/gdb/testsuite/gdb.base/reggroups.exp
> new file mode 100644
> index 0000000..fd51c50
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/reggroups.exp
> @@ -0,0 +1,71 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2014-2017 Free Software Foundation, Inc.

Should it be 2017 only?

> +
> +# 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 listing reggroups and the registers in each group.
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile 
> debug]} {
> +    return -1
> +}
> +
> +if ![runto_main] then {
> +    fail "can't run to main"
> +    return 0
> +}
> +
> +proc fetch_reggroups {test} {
> +    global gdb_prompt
> +    global expect_out
> +
> +    set reggroups {}
> +    set bad -1
> +    gdb_test_multiple "maint print reggroups" "get reggroups" {
> +	-re "maint print reggroups\r\n" {
> +	    exp_continue
> +	}
> +	-re "^ Group\[ \t\]+Type\[ \t\]+\r\n" {
> +	    exp_continue
> +	}
> +	-re "^ (\[0-9a-zA-Z\-\]+)\[ \t\]+(user|internal)\[ \t\]+\r\n" {
> +	    lappend reggroups $expect_out(1,string)
> +	    exp_continue
> +	}
> +	-re ".*$gdb_prompt $" {
> +	    if { [llength $reggroups] != 0 } {
> +		incr bad
> +	    } else {
> +		set bad 1
> +	    }

I don't quite understand this.  Why not simply "set bad 1"?

Another option would be to call error to throw an exception, which will 
end the test abruptly:

   error "maint print reggroups: unexpected output"

You could also assert that there is at least one reggroup with something 
like:

   gdb_assert { [llength $reggroups] > 0 } "at least one reggroup"

> +	}
> +    }
> +
> +    if {$bad} {
> +	fail $test
> +	return {}
> +    }
> 
> +
> +    pass $test
> +    return $reggroups
> +}
> +
> +set reggroups [fetch_reggroups "fetch reggroups"]
> +set reggroup ""

You don't need to declare reggroup.

> +
> +foreach reggroup [lrange $reggroups 0 end] {

And this can be simply:

   foreach reggroup $reggroups

> +    gdb_test "info reg $reggroup" ".*" "info reg $reggroup"
> +}

Simon