Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@polymtl.ca>
To: Petr Tesarik <ptesarik@suse.cz>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v2 3/4] Make sure that sorting does not change section order
Date: Tue, 26 Jun 2018 13:26:00 -0000	[thread overview]
Message-ID: <b65d3100ddda2b4709da3b3b5c5fd3b6@polymtl.ca> (raw)
In-Reply-To: <20180626070943.7331c1d2@ezekiel.suse.cz>

On 2018-06-26 01:09, Petr Tesarik wrote:
>> It took me a while to acknowledge that this was correct, because
>> other_sections::sectindex usually refers to the section index in the
>> BFD.  After digging I understood that this field was actually unused
>> until filled by addr_info_make_relative, and that you kind of
>> re-purposed it.  It sounds like there should be some comment at
>> other_sections::sectindex and probably in add_symbol_file_command to
>> explain how it's used.
> 
> Agreed. As a matter of fact, it also took me some while to understand
> why add_symbol_file_command could get away with setting the index to
> zero for all sections...
> 
>> Another option would be to use std::stable_sort instead of std::sort.
>> But it's more resource-hungry and not needed for all paths that lead 
>> to
>> addrs_section_sort, so it would be a bit wasteful.
> 
> Yes, I tried to avoid that solution. OTOH it's unlikely that there are
> any object files with more than a few dozen sections, and to my best
> knowledge this code is never in the GDB hot path, so if you prefer
> std::stable_sort for clarity, I'm not against. Please, advise.
> 
> Petr T

I think you solution is fine, it just needs to be documented that it's 
intentional that we use some other value than the section index in 
sectindex.

Simon


  reply	other threads:[~2018-06-26 13:26 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-11 12:08 [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
2018-06-11 12:08 ` [PATCH v2 3/4] Make sure that sorting does not change section order Petr Tesarik
2018-06-26  2:37   ` Simon Marchi
2018-06-26  5:10     ` Petr Tesarik
2018-06-26 13:26       ` Simon Marchi [this message]
2018-06-11 12:08 ` [PATCH v2 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
2018-06-11 15:23   ` Eli Zaretskii
2018-06-26  2:02   ` Simon Marchi
2018-06-11 12:08 ` [PATCH v2 2/4] Make add-symbol-file's address argument optional Petr Tesarik
2018-06-11 15:25   ` Eli Zaretskii
2018-06-11 16:50     ` Petr Tesarik
2018-06-11 17:25       ` Eli Zaretskii
2018-06-26  2:14   ` Simon Marchi
2018-06-11 12:23 ` [PATCH v2 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
2018-06-26  2:52   ` Simon Marchi
2018-06-22  7:52 ` [PATCH v2 0/4] Allow loading symbol files with an offset Petr Tesarik
2018-06-26  2:58   ` Simon Marchi
2018-06-26  5:01     ` Petr Tesarik
2018-06-26 15:15       ` Simon Marchi
2018-06-26 15:23         ` Petr Tesarik
2018-06-27 12:14 ` [PATCH v3 " Petr Tesarik
2018-06-27 12:14   ` [PATCH v3 3/4] Make sure that sorting does not change section order Petr Tesarik
2018-06-27 12:14   ` [PATCH v3 1/4] Add an optional offset option to the "symbol-file" command Petr Tesarik
2018-06-27 12:14   ` [PATCH v3 2/4] Make add-symbol-file's address argument optional Petr Tesarik
2018-06-27 12:33   ` [PATCH v3 4/4] Add an optional offset option to the "add-symbol-file" command Petr Tesarik
2018-06-27 12:57   ` [PATCH v3 0/4] Allow loading symbol files with an offset Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b65d3100ddda2b4709da3b3b5c5fd3b6@polymtl.ca \
    --to=simon.marchi@polymtl.ca \
    --cc=gdb-patches@sourceware.org \
    --cc=ptesarik@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox