public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Sergio Durigan Junior <sergiodj@redhat.com>
Cc: GDB Patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH v2] Make '{add-,}symbol-file' not care about the position of command line arguments
Date: Thu, 30 Nov 2017 12:49:00 -0000	[thread overview]
Message-ID: <b202e473-38f5-687e-eb37-e50cbe2174a0@redhat.com> (raw)
In-Reply-To: <87po807xhu.fsf@redhat.com>

On 11/30/2017 12:38 PM, Sergio Durigan Junior wrote:
> On Thursday, November 30 2017, Pedro Alves wrote:
> 
>> On 11/30/2017 04:24 AM, Sergio Durigan Junior wrote:
>>> Changes from v1:
>>>
>>> - Commit message has been rewritten.
>>>
>>> - Implemented position-independent argument parsing for
>>>   'add-symbol-file'.
>>>
>>> - Added testcases.
>>
>> Looks like you missed the comment about "--".  Take a look at
>> maintenance_print_symbols for an example of a command
>> that supports ending options with "--".  Can you add that
>> while you're at it, please?  For a test, I'd suggest
>> e.g., "symbol-file -- -non-existent-file" and confirming
>> gdb errors out.  That's simpler than actually creating a file.
> 
> Oh, I didn't understand that you were asking me to implement it.  I
> thought it was a comment for "something to be done later".  I'll do it,
> then.

I'd rather do it at the same time because there's the possibility
that implementing it suggests implementing any-position arguments
differently.  Or maybe not.  But I'd rather that we didn't have
to redo any-position support later on again.

>>
>> Does this push_back directly here mean that these
>> two commands end up with different semantics?
>>
>>  (gdb) add-symbol-file FILE 0 -s .text 0x1000
>>  (gdb) add-symbol-file -s .text 0x1000 FILE 0
> 
> The arguments are printed in a different order, yes:
> 
> (gdb) add-symbol-file -s .text 0x100 ./gdb/testsuite/outputs/gdb.base/relocate/relocate.o 0
> add symbol table from file "./gdb/testsuite/outputs/gdb.base/relocate/relocate.o" at
>         .text_addr = 0x100
>         .text_addr = 0x0
> (y or n) n
> Not confirmed.
> (gdb) add-symbol-file  ./gdb/testsuite/outputs/gdb.base/relocate/relocate.o 0 -s .text 0x100
> add symbol table from file "./gdb/testsuite/outputs/gdb.base/relocate/relocate.o" at
>         .text_addr = 0x0
>         .text_addr = 0x100
> (y or n) n
> Not confirmed.
> 
>> Not sure that's a good idea.
> 
> I'll work on it and make sure they're always printed in the same way.

TBC, the worry was not really about the printing (that's minor),
but what is the actual resulting .text address that the symbol file
it loaded at in the end.  But sounds like the printing order can
be used as proxy for checking that.  It's not as robust as
actually loading the symbols and then checking what happened
(with e.g., printing a symbol's address), but it's good
enough.

Thanks,
Pedro Alves

  reply	other threads:[~2017-11-30 12:49 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-06 20:54 [RFC/RFA] Add support for the --readnever command-line option (DWARF only) Joel Brobecker
2016-07-12 14:27 ` Yao Qi
2016-10-04 18:07   ` Pedro Alves
2017-11-23  0:54     ` [PATCH v2] " Sergio Durigan Junior
2017-11-23 12:09       ` Pedro Alves
2017-11-23 17:21         ` Sergio Durigan Junior
2017-11-23 17:29           ` Pedro Alves
2017-11-24  4:54             ` Sergio Durigan Junior
2017-11-24 13:18               ` Pedro Alves
2017-11-24 20:27                 ` Sergio Durigan Junior
2017-11-27 19:13                   ` Pedro Alves
2017-11-29  0:59                     ` Sergio Durigan Junior
2017-11-29 12:23                       ` Pedro Alves
2017-11-23 15:59       ` Eli Zaretskii
2017-11-23 19:36         ` Sergio Durigan Junior
2016-10-04 18:06 ` [RFC/RFA] " Pedro Alves
2017-11-24 23:01 ` [PATCH v2] Add support for the --readnever command-line option Sergio Durigan Junior
2017-11-25  7:33   ` Eli Zaretskii
2017-11-25 16:41     ` Sergio Durigan Junior
2017-11-25 17:16       ` Eli Zaretskii
2017-11-29  1:21 ` [PATCH v3] Add support for the readnever concept Sergio Durigan Junior
2017-11-29  3:39   ` Eli Zaretskii
2017-11-29 12:25   ` Pedro Alves
2017-11-29 18:43     ` Sergio Durigan Junior
2017-11-29 21:45     ` [PATCH] Make 'symbol-file' not care about the position of command line arguments Sergio Durigan Junior
2017-11-29 22:26       ` Pedro Alves
2017-11-29 22:42         ` Sergio Durigan Junior
2017-11-29 23:15           ` Pedro Alves
2017-11-30  0:08             ` Sergio Durigan Junior
2017-11-30  0:34               ` Pedro Alves
2017-11-30  4:07                 ` Sergio Durigan Junior
2017-11-30  4:25       ` [PATCH v2] Make '{add-,}symbol-file' " Sergio Durigan Junior
2017-11-30 10:57         ` Pedro Alves
2017-11-30 12:38           ` Sergio Durigan Junior
2017-11-30 12:49             ` Pedro Alves [this message]
2017-11-30 13:06               ` Sergio Durigan Junior
2017-11-30 13:33       ` [PATCH v3] " Sergio Durigan Junior
2017-11-30 15:01         ` Pedro Alves
2017-11-30 17:26           ` Sergio Durigan Junior
2017-11-30 17:37             ` Pedro Alves
2017-11-30 17:43               ` Sergio Durigan Junior
2017-11-30 17:50                 ` Pedro Alves
2017-11-30 20:00       ` [PATCH v4] " Sergio Durigan Junior
2017-12-01 12:11         ` Pedro Alves
2017-12-01 17:41           ` Sergio Durigan Junior
2017-12-01 21:45             ` Pedro Alves
2017-12-01 22:02               ` Sergio Durigan Junior
2017-11-30  0:25 ` [PATCH v4] Add support for the readnever concept Sergio Durigan Junior
2017-11-30 11:53   ` Pedro Alves
2017-12-01  4:35     ` Sergio Durigan Junior
2017-12-01 12:43       ` Pedro Alves
2017-12-01 17:19         ` Tom Tromey
2017-12-01 17:21           ` Sergio Durigan Junior
2017-12-01 20:00             ` Pedro Alves
2017-12-01 22:16 ` [PATCH v5] " Sergio Durigan Junior
2017-12-01 23:19   ` Pedro Alves
2017-12-02  2:31     ` Sergio Durigan Junior
2017-12-02  8:21   ` Eli Zaretskii

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=b202e473-38f5-687e-eb37-e50cbe2174a0@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=sergiodj@redhat.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).