Re: [PATCH v3 2/5] btrace: allow recording to be started (and stopped) for running threads

[Pedro Alves <palves at redhat dot com>]

On 01/30/2017 10:05 AM, Markus Metzger wrote:

> ---
>  gdb/btrace.c                                | 46 ++++++++++++++--
>  gdb/testsuite/gdb.btrace/enable-running.c   | 47 ++++++++++++++++
>  gdb/testsuite/gdb.btrace/enable-running.exp | 85 +++++++++++++++++++++++++++++
>  3 files changed, 174 insertions(+), 4 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.btrace/enable-running.c
>  create mode 100644 gdb/testsuite/gdb.btrace/enable-running.exp
> 
> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index d266af7..fec2cce 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -141,6 +141,18 @@ ftrace_debug (const struct btrace_function *bfun, const char *prefix)
>  		prefix, fun, file, level, ibegin, iend);
>  }
>  
> +/* Validate that we can read PTID's registers.  */
> +
> +static void
> +validate_registers_access_ptid (ptid_t ptid)
> +{
> +  struct cleanup *cleanup = save_inferior_ptid ();
> +
> +  inferior_ptid = ptid;
> +  validate_registers_access ();
> +  do_cleanups (cleanup);
> +}
> +

Please let's refactor things a bit to avoid the need to frob
inferior_ptid and restore with a cleanup.  Change the existing
validate_registers_access like:

  static void
 -validate_registers_access ()
 +validate_registers_access_ptid (ptid_t ptid)
  {
   /* No selected thread, no registers.  */
   -if (ptid_equal (inferior_ptid, null_ptid))
   +if (ptid_equal (ptid, null_ptid))
     error (_("No thread selected."));
   [etc.]

and then reimplement it back on top of validate_registers_access_ptid:

validate_registers_access ()
{
  return validate_registers_access_ptid (inferior_ptid);
}

> +
> +#include <pthread.h>
> +
> +#define NTHREADS 3
> +
> +static void *
> +test (void *arg)
> +{
> +  /* Let's hope this is long enough for GDB to enable tracing and check that
> +     everything is working as expected.  */
> +  sleep (10);

Need to include <unistd.h> for sleep.  I got:

..../src/gdb/testsuite/gdb.btrace/enable-running.c: In function 'test':
..../src/gdb/testsuite/gdb.btrace/enable-running.c:27:3: warning: implicit declaration of function 'sleep' [-Wimplicit-function-declaration]
   sleep (10);
   ^

Otherwise LGTM.

Thanks,
Pedro Alves