RE: [PATCH v2 5/7] btrace, gdbserver: remove the to_supports_btrace target method

Maciej W. Rozycki
2018-02-27T15:32:03Z

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