[PATCH] Change section_offsets to a std::vector

Tom Tromey tromey@adacore.com
Wed Jan 8 22:11:00 GMT 2020


>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I think that's great, it improves readability a lot.  Also, after having read
Simon> this patch, I kinda want to make a helper method on objfile to replace this
Simon> pattern:

objfile-> section_offsets[SECT_OFF_TEXT (objfile)];

Simon> with

objfile-> text_section_offset ()

Simon> It happens really often, and I think it would be even easier to read.

Yeah, that would be nice.

>> -  offs = ((struct section_offsets *)
>> -	  alloca (SIZEOF_N_SECTION_OFFSETS (symfile_objfile->num_sections)));
>> -  memcpy (offs, symfile_objfile->section_offsets,
>> -	  SIZEOF_N_SECTION_OFFSETS (symfile_objfile->num_sections));
>> +  section_offsets offs (symfile_objfile->section_offsets.size ());

Simon> Is something missing here?  The existing code copies the content of
symfile_objfile-> section_offsets into "offs", but the new code doesn't.
Simon> Perhaps it should copy the vector?
Simon>     section_offsets offs = symfile_objfile->section_offsets;
Simon> ?  I don't really know what the code does, so I can't tell for sure.

Good catch, thank you very much.  I made this change.

...
Simon> The last part of the comment here could be removed.

Fixed.

>> Each struct contains an array of offsets.
>> The ordering and meaning of the offsets is file-type-dependent;
>> typically it is indexed by section numbers or symbol types or
>> -   something like that.
>> +   something like that.  */

Simon> The "Each struct" part of the comment sounds like it could be removed.

Thanks, I made this change as well.

I'm going to check this in shortly.

Tom



More information about the Gdb-patches mailing list