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

[Jan Kratochvil <jan.kratochvil@redhat.com>]

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.


> > static int
> > symfile_target_read_memory (CORE_ADDR memaddr, gdb_byte *myaddr, int
> > len) {
> >   return target_read_memory (memaddr, myaddr, len);
> > }
> > 
> > But I proposed to rather target_read_memory use size_t and we should
> > then proposed to bfd/ that it also uses size_t for
> > bfd_elf_bfd_from_remote_memory.
> 
> Perhaps it will be helpful if I keep this change (and consequently the
> rest of the read/write_memory function changes) as a separate patch?

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.



Thanks,
Jan