Re: bitpos expansion patches summary

Jan Kratochvil
2012-08-27T14:02:49Z

On Mon, 27 Aug 2012 10:09:27 +0200, Siddhesh Poyarekar wrote:
> On Sun, 26 Aug 2012 19:43:35 +0200, Jan wrote:
> > (i386-nat.c:613):
> > FUNC(i386_length_and_rw_bits): (LONGEST to int) [len]
> > - To be re-checked with updated watchpoints patch.
>
> This is safe, since ok_for_watchpoint will restrict the incoming LEN in
> insert_watchpoint.
>
> > WPREVERTED(Likewise): (i386-nat.c:647):
> > FUNC(i386_length_and_rw_bits): (LONGEST to int) [len]
> > - To be re-checked with updated watchpoints patch.
>
> Likewise for remove_watchpoint.

OK, I agree with these two, these are already i386-specific and i386 (contrary
to s390) has very limited maximum span of a single hardward watchpoint.

> Yes, all of the ENSURED_SIZET for -tdep have been removed for similar
> reasons.

It will be fun to merge all the pending patches and reviews in the end.

> > SAFE:
> > (mdebugread.c:1238): ASSIGN: (bfd_vma to
> > LONGEST) [ ((*f).loc.bitpos) = (sh->value)]
> > - Not so clear to me, --disable-64-bit-bfd cannot support >=2GB
> > inferiors but it should support >256MB structs. Still when it also
> > affects only STABS I do not think it needs a fix.
>
> Either ways, the point is that sizeof (bfd_vma) would always be less or
> equal to than sizeof (LONGEST) and hence this ought to be safe.

OK, I agree, I have mistaken my review here.

Thanks,
Jan