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

[Siddhesh Poyarekar <siddhesh@redhat.com>]

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