Re: [PATCH, AArch64] Fix bug in hardware watchpoint/breakpoint handling

[pinskia@gmail.com]

> On Dec 19, 2013, at 2:16 AM, Marcus Shawcroft <marcus.shawcroft@gmail.com> wrote:
> 
>> On 19 December 2013 07:00,  <pinskia@gmail.com> wrote:
>> I think this patch is wrong as the size that is passed is always just one element as sizeof (regs.dbg_regs [count - 1]) is the same as sizeof (regs.dbg_regs [0]). This should have been sizeof (regs.dbg_regs [0])*count instead.
> 
> 
> +  iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs[count - 1])
> + + sizeof (regs.dbg_regs [count - 1]));
> 
> The offsetof() gives the number of bytes from the start of
> user_hwdebug_state upto and including all of the reg array bar the
> last in use entry.  Adding on the sizeof() therefore gives the number
> of bytes from the start of the structure upto and including the last
> in use reg entry.
> 
> I agree that the sizeof() could be written either with dbg_regs[0] or
> dbg_regs[count-1] with no change in behavior.  But I think Yufengs
> code is functionally correct.
> 
> It would have been simpler to write:
> iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs[count])
> 
> .. but I think that is illegal when count equals the number of
> elements defined in the array.


Oh, I see I think this should have been written using offsetof [0] and then count*sizeof to make easier to understand and fits in with most other code which uses a variable size last element. 

Thanks,
Andrew 

> 
> Cheers
> /Marcus