Re: [PATCH v2 0/7] improve btrace enable error reporting

Pedro Alves
2018-02-06T16:28:12Z

Hi Markus,

On 01/26/2018 02:14 PM, Markus Metzger wrote:
> Recording may fail for a variety of reasons. Improve the error
> messages by stating more clearly what operation failed and try to give
> a reason why it failed.
>
> Further align the error messages for native and remote debugging.
>
> Changes to v1:
> - move helper classes into gdb/common/
> - add unit tests for helpers
> - simplify helpers
>
> Markus Metzger (7):
> common: add scoped_fd
> common: add scoped_mmap
> btrace: prepare for throwing exceptions when enabling btrace
> btrace, gdbserver: use exceptions to convey btrace enable/disable
> errors
> btrace, gdbserver: remove the to_supports_btrace target method
> btrace: improve enable error messages
> btrace: check perf_event_paranoid

This LGTM, though I have a couple questions, and a nit.

#1 - Where does this leave up wrt to:

'old gdb' x 'new gdbserver'
and
'new gdb' x 'old gdbserver'

?

#2 - Where we now say

+ error (_("GDB does not support Intel PT."));

(and similarly for BTS)

shouldn't that say something like "_This_ GDB does not",
so that the user can tell that it's a matter of that
particular build of gdb, not that GDB-the-project is lacking
support for PT?

#3 - in patch 7:

Instead of:

const char *filename = "/proc/sys/kernel/perf_event_paranoid";

write:

static const char filename[] = ...;

Thanks,
Pedro Alves