Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Andreas Arnez <arnez@linux.ibm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Don't print symbol definition's line number in rbreak output
Date: Tue, 17 Apr 2018 15:18:00 -0000	[thread overview]
Message-ID: <4eae39d3-c6ca-ef5b-4127-0ad29ee792c9@redhat.com> (raw)
In-Reply-To: <m3k1t63tla.fsf@oc1027705133.ibm.com>

On 04/17/2018 01:28 PM, Andreas Arnez wrote:
> On Mon, Apr 16 2018, Pedro Alves wrote:
> 
>> On 04/16/2018 07:44 PM, Andreas Arnez wrote:
>>> This commit:
>>>
>>>   b744723f57 -- Show line numbers in output for "info var/func/type"
>>>
>>> added the symbol definition's line number to the output of certain GDB
>>> commands.  It also changes the `rbreak' command's output, although it
>>> shouldn't.  This is fixed.
>>
>> Could you update this to include an example of before/after gdb
>> output in the commit log?
> 
> Sure.  How about the updated commit message below?
> 

Thanks.

>>
>> Is this a regression in 8.1?
> 
> No, I just caused the regression myself on Friday with the commit above.
> For some reason I had not noticed the impact on the `rbreak' command
> before.

Ahah, thanks, it's much clearer now.

> No declaration line number was shown before.  Instead, the function
> declaration started at the first column.  This old behavior is restored.

IMHO it's always better to also paste the output after, so the
reader doesn't have to imagine it from the description.

>  /* Helper function for symtab_symbol_info, this function uses
>     the data returned from search_symbols() to print information
> -   regarding the match to gdb_stdout.  */
> +   regarding the match to gdb_stdout.  If LAST is not NULL,
> +   print file- and line number information for the symbol as
> +   well.  Skip printing the filename if it matches LAST.  */

Nit, I don't think use of suspense hyphen in this case is common
in English: <http://grammartips.homestead.com/hyphens3.html>.
I'd just write plain "file".

The patch looks fine to me.  However, I notice now that this doesn't
tweak any existing testcase or add any new one.  Was the problem
caught by some existing testcase?

Thanks,
Pedro Alves


  reply	other threads:[~2018-04-17 15:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-16 18:44 Andreas Arnez
2018-04-16 19:01 ` Pedro Alves
2018-04-17 12:28   ` Andreas Arnez
2018-04-17 15:18     ` Pedro Alves [this message]
2018-04-17 16:39       ` Andreas Arnez
2018-04-17 16:56         ` Pedro Alves
2018-04-17 17:33           ` Andreas Arnez

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=4eae39d3-c6ca-ef5b-4127-0ad29ee792c9@redhat.com \
    --to=palves@redhat.com \
    --cc=arnez@linux.ibm.com \
    --cc=gdb-patches@sourceware.org \
    /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