Re: [RFA]: Change to_stopped_data_address ABI

Eli Zaretskii
2004-08-31T19:38:37Z

> Date: Tue, 31 Aug 2004 14:55:13 -0400
> From: Jeff Johnston <jjohnstn@redhat.com>
>
> The proposed change is to change the prototype to be:
>
> int
> to_stopped_data_address (CORE_ADDR *addr_p);
>
> If the input pointer is NULL, the function returns non-zero if it works on the
> given target, otherwise, it fails by returning 0. The function also returns 0
> if unsuccessful. By separating out the success/fail code from the address, the
> new prototype allows for succeeding and returning any address, including 0.

Thanks.

The idea is okay with me, but the code tells a bit different story
(unless I missed something, in which case I apologize). From your
description, I initially understood that you want to allow to return a
zero address when the watchpoint triggers at that address. For that,
if no watchpoint triggered, to_stopped_data_address will return zero
as its value, not put a NULL pointer into a place pointed to by its
argument. That would be okay with me, but your code does something
different:

> @@ -2739,8 +2739,7 @@ bpstat_stop_status (CORE_ADDR bp_addr, p
> struct value *v;
> int found = 0;
>
> - addr = target_stopped_data_address ();
> - if (addr == 0)
> + if (!target_stopped_data_address (&addr))
> continue;

This seems to say that target_stopped_data_address indeed returns a
zero value for the case where no watchpoint triggered...

> +int
> +i386_stopped_data_address (CORE_ADDR *addr_p)
> {
> CORE_ADDR addr = 0;
> int i;
>
> + if (addr_p == NULL)
> + return 1;
> +
> dr_status_mirror = I386_DR_LOW_GET_STATUS ();
>
> ALL_DEBUG_REGISTERS(i)
> @@ -593,7 +598,16 @@ i386_stopped_data_address (void)
> if (maint_show_dr && addr == 0)
> i386_show_dr ("stopped_data_addr", 0, 0, hw_write);
>
> - return addr;
> + *addr_p = addr;
> + return 1;

...but this returns 1 as the function's value and puts zero where the
argument points. Isn't that a contradiction? And doesn't this code
in i386_stopped_data_address still disallow support for a watchpoint
at address zero by retaining the previous magic meaning of a zero
address? Or did I miss something?

> +zero. When @var{addr_p} is non-NULL, return non-zero if the data address
> +for the triggered watchpoint is determined successfully, otherwise, return zero.

I think "watchpoint is determined successfully" is not clear enough.
Please rewrite the text to say exactly what does the zero value mean.
The intent is to tell a GDB hacker how to handle the case of zero
return value from target_stopped_data_address.