Re: [RFA] Fix verification of changed values for big values.

Joel Brobecker
2009-12-24T04:41:52Z

> But I don't know if approvals expire or not, so...

I guess it depends whether the code that you're updating has changed
much or not. If not, I'd say that the approval does not expire.

> gdb/
> * valarith.c (value_equal_contents): New function.
> * value.h (value_equal_contents): Declare.
> * breakpoint.c (watchpoint_check): Use value_equal_contents
> instead of value_equal.

OK, with just one little request.

>
> gdb/testsuite/
> * gdb.base/watchpoint.exp (test_watchpoint_in_big_blob): New function.
> (top level): Call test_watchpoint_in_big_blob.
> * gdb.base/watchpoint.c (buf): Change size to value too big for hardware
> watchpoints.
> (func3): Write to buf.

Just one comment, but pre-approved.

> -/* Check watchpoint condition. */
>
> static int
> watchpoint_check (void *p)

Can you add a short description of what the function does? We would
like all functions to be documented... In particular, since P is
declared as a void *, it's probably going to be useful to explain
what the real type is supposed to be.

> +/* Compare values based on their raw contents. Useful for arrays since
^ Missing space

> + send_gdb "cont\n"
> + gdb_expect {
> + -re "Continuing.*\[Ww\]atchpoint.*buf.*Old value = .*$gdb_prompt $" {
> + pass "watchpoint on buf hit"
> + }

I am wondering if this could be written more simply, by using gdb_test?

gdb_test "cont" "Continuing.*\[Ww\]atchpoint.*buf.*Old value = .*" [...]

? Otherwise, we try to avoid the use of send_gdb/gdb_expect, as it
forces you to handle by hand all possible failure conditions. You can
use gdb_test_multiple instead. For instance:

| gdb_test_multiple "next" "next after watch x" {
| -re ".*atchpoint \[0-9\]+: x\r\n\r\nOld value = 0\r\nNew value = 1\r\n.*$gdb_prompt $" {
| pass "next after watch x"
| }
| -re "\[0-9\]+\[\t \]+y = 1;\r\n$gdb_prompt $" {
| kfail "gdb/38" "next after watch x"
| }

(notice how you know longer need to handle eof, timeout, but also
internal errors, etc)

--
Joel