On Thu 24 Dec 2009 02:41:25 Joel Brobecker wrote:
> > 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.

Thanks!

> > -/* 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.

The purpose of the function is rather simple, so I didn't have much to say.
I added this description:

/* Evaluate watchpoint condition expression and check if its value changed.  */

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

Fixed.

> > +    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

You're right. IIRC I used gdb_expect because I wanted to explicitly check
for the gdb prompt in my regex, to avoid it being too greedy. But I changed
the testcase a little bit and now I don't need that anymore.

This is what I committed.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


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.

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.