Re: [PATCH 1/3] Add testcases for examine function

[Joel Brobecker <brobecker@adacore.com>]

On Wed, Nov 04, 2015 at 03:09:45PM +0800, Fei Jie wrote:
>     use examine to display the content of main function's address
>     in different formats and examine address that can not be accessed

As explained in our submission procedures, this change is missing
a ChangeLog entry.

> diff --git a/gdb/testsuite/gdb.base/testexamine.exp b/gdb/testsuite/gdb.base/testexamine.exp
> new file mode 100644
> index 0000000..16d916d
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/testexamine.exp

*ALL* new files must have a copyright header. For .exp files, suggest
you just copy it from another file, and fix the copyright year as
appropriate (year range from when the file was first saved on
hard drive).

> @@ -0,0 +1,42 @@
> +if {[prepare_for_testing testprint.exp testprint display.c \
> +	{debug nowarnings}]} {
> +	untested testprint.exp
> +	return -1
> +
> +if ![runto_main] then {
> +	fail "Can not run to main."
> +}

As mentioned in a previous message, please take a look at how
we prefer tests to be built:

https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Building_the_Example_Program

In particular, use "standard_testfile", and lose the untested.
See also the block on we "runto_main" (we don't use "fail" in this
case).

> +#Get main address

We follow the GNU Coding Standards (GCS):
http://www.gnu.org/prep/standards/standards.html

So, could you please add a space after the "#", and also terminate
all sentences with a period?

# Get main's address.

> +set main_addr ""
> +gdb_test_multiple "print/x &main" "getting main's address" {
> +    -re "$decimal = \($hex\)\r\n$gdb_prompt $" {
> +        set main_addr $expect_out(1,string)
> +    }
> +}
> +
> +#Test x(examine)
> +gdb_test "x" \
> +	"Argument required.*"
> +gdb_test "x/x $main_addr" \
> +	".*<main>.*0xe5894855"

I am afraid that this isn't going to work for anyone by you,
and only when using your very specific compiler. You're expecting
the compiler to compile the code in such a way that the first
word of the function is 0xe5894855, which is not always true.

Better, IMO, to do this using a variable, either global or local,
whose content is well known.

> +gdb_test "x 0x000000000000" \
> +	"Cannot access memory at address 0x0"

This, unfortunately, is not guaranteed to work either. Some targets
allow reading at this address. I would just remove that test.

> +gdb_exit
> +return 0

This is also unnecessary.

-- 
Joel