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

Jan Kratochvil
2012-05-23T17:46:29Z

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