Re: [PATCH v2] Expand bitpos and type.length to LONGEST and ULONGEST

Siddhesh Poyarekar
2012-05-24T01:36:26Z

On Wed, 23 May 2012 19:46:10 +0200, Jan wrote:

> Hi Siddhesh,
>
> On Wed, 23 May 2012 15:52:45 +0200, Siddhesh Poyarekar wrote:
> > > > static void
> > > > -copy_bitwise (gdb_byte *dest, unsigned int dest_offset_bits,
> > > > - const gdb_byte *source, unsigned int
> > > > source_offset_bits,
> > > > - unsigned int bit_count,
> > > > +copy_bitwise (gdb_byte *dest, ULONGEST dest_offset_bits,
> > > > + const gdb_byte *source, ULONGEST source_offset,
> > > > + ULONGEST bit_count,
> > > > int bits_big_endian)
> > > > {
> > > > - unsigned int dest_avail;
> > > > + unsigned int dest_avail, source_offset_bits;
> > > > int datum;
> > > >
> > > > /* Reduce everything to byte-size pieces. */
> > > > dest += dest_offset_bits / 8;
> > > > dest_offset_bits %= 8;
> > > > - source += source_offset_bits / 8;
> > > > - source_offset_bits %= 8;
> > > > + source += source_offset / 8;
> > > > + source_offset_bits = source_offset % 8;
> > >
> > > I do not fully understand this whole change but it looks
> > > unrelated to this patch to me.
> >
> > I had to split up the source_offset and source_offset bits because
> > source_offset can be LONGEST but the bits would always be less than
> > 8. I did this because otherwise I would have had to change
> > signatures of functions that use source_offset_bits even when it is
> > not really needed (extract_bits and extract_bits_primitive).
>
> OK, understood now. Therefore just to make splint quiet.
>
> Just in such case - without running splint now myself -
> dest_offset_bits seems to have exactly the same problem, it is passed
> to insert_bits.

dest_offset_bits is safe to be truncated when passing into insert_bits.
The trouble with source_offset_bits is that its pointer is passed into
extract_bits, because of which the function signature change becomes
non-optional unless just a cast is OK here.

> Yes, this size_t change - needs to be also posted to
> binutils@sourceware.org for approval (the bfd/ part). I think it
> should be done first and the mail to binutils@sourceware.org (Cc
> gdb-patches) should not contain parts affecting only gdb/ (it should
> contain parts needed to keep gdb/ compatible with the bfd/ changes).
> Also maybe binutils will have different opinion on it.

OK, thanks for letting me know. I'll do this first.

Regards,
Siddhesh