This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gold patch] Decompress sections in Read_symbols task


>> @@ -62,12 +62,15 @@ ResetLineStateMachine(struct LineStateMachine* lsm, bool default_is_stmt)
>> ?}
>>
>> ?template<int size, bool big_endian>
>> -Sized_dwarf_line_info<size, big_endian>::Sized_dwarf_line_info(Object* object,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int read_shndx)
>> - ?: data_valid_(false), buffer_(NULL), symtab_buffer_(NULL),
>> - ? ?directories_(), files_(), current_header_index_(-1)
>> +Sized_dwarf_line_info<size, big_endian>::Sized_dwarf_line_info(
>> + ? ?Object* object,
>> + ? ?unsigned int read_shndx)
>> + ?: data_valid_(false), buffer_(NULL), buffer_start_(NULL),
>> + ? ?symtab_buffer_(NULL), directories_(), files_(), current_header_index_(-1)
>> ?{
>> ? ?unsigned int debug_shndx;
>> + ?bool is_new = false;
>> +
>
> It looks like is_new is only used within a block, so bring the
> declaration down into that block.

Done.

>> @@ -550,8 +550,22 @@ Sized_relobj_file<size, big_endian>::find_eh_frame(
>> ? ?return false;
>> ?}
>>
>> +// Return TRUE if this is a section whose contents will be needed in the
>> +// Add_symbols task.
>> +
>> +bool
>> +need_decompressed_section(const char* name)
>> +{
>> + ?// We will need .zdebug_str if this is not an incremental link
>> + ?// (i.e., we are processing string merge sections).
>> + ?if (!parameters->incremental() && strcmp(name, ".zdebug_str") == 0)
>> + ? ?return true;
>> +
>> + ?return false;
>> +}
>
> Make this function static.

Done.

>> + ?section_size_type uncompressed_size = p->second.size;
>> + ?if (p->second.contents != NULL)
>> + ? ?{
>> + ? ? ?*plen = uncompressed_size;
>> + ? ? ?*is_new = false;
>> + ? ? ?return p->second.contents;
>> + ? ?}
>> +
>> + ?unsigned char* uncompressed_data = new unsigned char[uncompressed_size];
>> + ?if (!decompress_input_section(buffer,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? buffer_size,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? uncompressed_data,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? uncompressed_size))
>> + ? ?this->error(_("could not decompress section %s"),
>> + ? ? ? ? ? ? this->do_section_name(shndx).c_str());
>> +
>> + ?*plen = uncompressed_size;
>> + ?*is_new = true;
>> + ?return uncompressed_data;
>
> Why not
> ? ?p->second.contents = uncompressed_data;
> ? ?*is_new = false;
> ? ?I think it's because you are assuming that if it was not done in
> build_compressed_section_map, it will not be profitable here. ?In that
> case, there should be a comment here.

Yes, you're right. I've added a comment.

>> + ?// Return a view of the decompressed contents of a section. ?Set *PLEN
>> + ?// to the size. ?This default implementation simply returns the
>> + ?// raw section contents and sets *IS_NEW to false to indicate
>> + ?// that the contents do not need to be freed by the caller.
>
> This comment needs a sentence explaining why it is OK to not decompress
> the section here.

Done.

> OK with those changes.

Thanks, committed.

-cary


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]