Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@mips.com>
To: "Metzger, Markus T" <markus.t.metzger@intel.com>
Cc: Andreas Arnez <arnez@linux.vnet.ibm.com>,
	"gdb-patches@sourceware.org"	<gdb-patches@sourceware.org>
Subject: RE: [PATCH v2 5/7] btrace, gdbserver: remove the to_supports_btrace target method
Date: Tue, 27 Feb 2018 15:32:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1802271409010.3553@tp.orcam.me.uk> (raw)
In-Reply-To: <A78C989F6D9628469189715575E55B236964C660@IRSMSX104.ger.corp.intel.com>

Hi Markus,

> https://sourceware.org/ml/gdb-patches/2018-02/msg00420.html
> 
> 
> It's the same I sent inline to the email with a recurring white-space 
> error fixed.

 With the change description you have also overrun our 74-column limit: 
<https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Column_limits>. 
NB due to how `git log' etc. indents descriptions I prefer to stay within 
72 columns with my own changes for a better visual effect, though you are 
of course free to use your own judgement here as long as you're within 74 
columns.

 Same with ChangeLog entries.  Also as per the the GNU Coding Standard:
<https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs>
long function lists, etc. use `)' as the closing character, so:

	* target.h (target_enable_btrace, target_disable_btrace)
	(target_read_btrace, target_read_btrace_conf): Turn macro into 
	inline function.  Throw error if target method not defined.

 You need to mark new error messages for translation, so:

+    error (_("Target does not support branch tracing."));

etc.

 This:

+static inline int
+target_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer,
+		    enum btrace_read_type type)

also overruns the 74-column limit.

 The formatting appears otherwise OK to me and I'll leave the matter 
itself of this change up to someone with suitable expertise in this area.

> Andreas already indicated that the patch fixes the problem on s390x.  Did the full
> test show any further issues on your side, Maciej?

 We had a failure last night in the lab literally as I have scheduled the 
tests and consequently they didn't run.  I'll schedule them again as soon 
as possible, but the target system I have been using for testing is not 
back up yet, so it may not happen before tomorrow.  Sorry.

> >  Also did you verify that old-GDB/new-gdbserver and new-GDB/old-gdbserver
> > combinations work correctly?
> 
> That was discussed here:
> https://sourceware.org/ml/gdb-patches/2018-02/msg00117.html

 Thanks for confirming.  Is there going to be any difference here for 
non-x86 targets that needs to be verified?

  Maciej


  reply	other threads:[~2018-02-27 15:32 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-26 14:14 [PATCH v2 0/7] improve btrace enable error reporting Markus Metzger
2018-01-26 14:14 ` [PATCH v2 2/7] common: add scoped_mmap Markus Metzger
2018-01-26 14:14 ` [PATCH v2 6/7] btrace: improve enable error messages Markus Metzger
2018-01-26 14:14 ` [PATCH v2 4/7] btrace, gdbserver: use exceptions to convey btrace enable/disable errors Markus Metzger
2018-01-26 14:14 ` [PATCH v2 5/7] btrace, gdbserver: remove the to_supports_btrace target method Markus Metzger
2018-02-24 16:51   ` Maciej W. Rozycki
2018-02-26 13:08     ` Metzger, Markus T
2018-02-26 19:30       ` Andreas Arnez
2018-02-26 21:58         ` Maciej W. Rozycki
2018-02-27 10:57           ` Metzger, Markus T
2018-02-27 15:32             ` Maciej W. Rozycki [this message]
2018-02-27 18:14               ` Metzger, Markus T
2018-02-28 10:29                 ` Maciej W. Rozycki
2018-02-28 11:09                   ` Maciej W. Rozycki
2018-02-28 11:24                     ` Metzger, Markus T
2018-02-28 12:53                       ` Metzger, Markus T
2018-02-28 15:55                         ` Maciej W. Rozycki
2018-02-28 16:08                         ` Eli Zaretskii
2018-02-28 12:00     ` Yao Qi
2018-02-28 16:27       ` Sergio Durigan Junior
2018-03-01 11:33         ` Metzger, Markus T
2018-03-01 19:27           ` Sergio Durigan Junior
2018-03-05 12:14             ` Yao Qi
     [not found]               ` <87lgf64i24.fsf@redhat.com>
2018-03-06  9:02                 ` Yao Qi
2018-03-06 16:32                   ` Sergio Durigan Junior
2018-03-01 19:34           ` Maciej W. Rozycki
2018-01-26 14:14 ` [PATCH v2 3/7] btrace: prepare for throwing exceptions when enabling btrace Markus Metzger
2018-01-26 14:14 ` [PATCH v2 1/7] common: add scoped_fd Markus Metzger
2018-02-13 16:48   ` Yao Qi
2018-02-13 17:28     ` Metzger, Markus T
2018-02-14 15:22       ` Yao Qi
2018-02-14 17:26         ` Metzger, Markus T
2018-02-19 15:28           ` Metzger, Markus T
2018-02-20 10:12             ` Yao Qi
2018-02-20 10:46               ` Metzger, Markus T
2018-02-06 16:28 ` [PATCH v2 0/7] improve btrace enable error reporting Pedro Alves
2018-02-07 10:41   ` Metzger, Markus T
2018-02-07 12:11     ` Pedro Alves
2018-02-08 10:40       ` Metzger, Markus T
     [not found]         ` <9ce0c90a-5c71-0a7d-3779-f826369a95ec@redhat.com>
2018-02-08 13:49           ` Metzger, Markus T

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=alpine.DEB.2.00.1802271409010.3553@tp.orcam.me.uk \
    --to=macro@mips.com \
    --cc=arnez@linux.vnet.ibm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=markus.t.metzger@intel.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