Re: [PATCH v3 2/8] btrace: Export btrace_decode_error function.

Re: [PATCH v3 2/8] btrace: Export btrace_decode_error function.

[Doug Evans]

Tim Wiederhake writes:
  > 2016-11-21  Tim Wiederhake  <tim.wiederhake@intel.com>
  >
  > gdb/ChangeLog:
  >
  > 	* record-btrace.c (btrace_ui_out_decode_error): Move most of it ...
  > 	* btrace.c (btrace_decode_error): ... here.  New function.
  > 	* btrace.h (btrace_decode_error): New export.

Hi.
LGTM
(modulo anything Markus or someone else wrote,
I read the comments but have since forgotten)

[PATCH v3 0/8] Python bindings for btrace recordings

[Tim Wiederhake]

This patch series adds Python bindings for btrace recordings.

V1 of this series can be found here:
https://sourceware.org/ml/gdb-patches/2016-10/msg00733.html

V2 of this series can be found here:
https://sourceware.org/ml/gdb-patches/2016-11/msg00084.html

Changes in V3:
- Rebased to current master.
- Replaced some strncmp with strcmp in patch 4 ("Add record_start function").
- Incorporated Eli's feedback regarding the documentation part.

Tim Wiederhake (8):
  btrace: Count gaps as one instruction explicitly.
  btrace: Export btrace_decode_error function.
  btrace: Use binary search to find instruction.
  Add record_start function.
  python: Create Python bindings for record history.
  python: Implement btrace Python bindings for record history.
  python: Add tests for record Python bindings
  Add documentation for new instruction record Python bindings.

 gdb/Makefile.in                               |   4 +
 gdb/NEWS                                      |   4 +
 gdb/btrace.c                                  | 163 +++--
 gdb/btrace.h                                  |  21 +-
 gdb/doc/python.texi                           | 237 ++++++
 gdb/python/py-btrace.c                        | 994 ++++++++++++++++++++++++++
 gdb/python/py-btrace.h                        |  32 +
 gdb/python/py-record.c                        | 257 +++++++
 gdb/python/py-record.h                        |  57 ++
 gdb/python/python-internal.h                  |   7 +
 gdb/python/python.c                           |  14 +
 gdb/record-btrace.c                           | 131 ++--
 gdb/record-full.c                             |  20 +
 gdb/record.c                                  |  28 +
 gdb/record.h                                  |   5 +
 gdb/target-debug.h                            |   2 +
 gdb/target-delegates.c                        |  33 +
 gdb/target.c                                  |   7 +
 gdb/target.h                                  |  10 +
 gdb/testsuite/gdb.python/py-record-btrace.c   |  48 ++
 gdb/testsuite/gdb.python/py-record-btrace.exp | 160 +++++
 21 files changed, 2096 insertions(+), 138 deletions(-)
 create mode 100644 gdb/python/py-btrace.c
 create mode 100644 gdb/python/py-btrace.h
 create mode 100644 gdb/python/py-record.c
 create mode 100644 gdb/python/py-record.h
 create mode 100644 gdb/testsuite/gdb.python/py-record-btrace.c
 create mode 100644 gdb/testsuite/gdb.python/py-record-btrace.exp

-- 
2.7.4

[PATCH v3 2/8] btrace: Export btrace_decode_error function.

[Tim Wiederhake]

2016-11-21  Tim Wiederhake  <tim.wiederhake@intel.com>

gdb/ChangeLog:

	* record-btrace.c (btrace_ui_out_decode_error): Move most of it ...
	* btrace.c (btrace_decode_error): ... here.  New function.
	* btrace.h (btrace_decode_error): New export.


---
 gdb/btrace.c        | 49 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/btrace.h        |  5 +++++
 gdb/record-btrace.c | 59 ++++-------------------------------------------------
 3 files changed, 58 insertions(+), 55 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 7df1b00..ac7d46e 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1699,6 +1699,55 @@ btrace_maint_clear (struct btrace_thread_info *btinfo)
 
 /* See btrace.h.  */
 
+const char *
+btrace_decode_error (enum btrace_format format, int errcode)
+{
+  switch (format)
+  {
+    case BTRACE_FORMAT_BTS:
+      switch (errcode)
+      {
+	case BDE_BTS_OVERFLOW:
+	  return _("instruction overflow");
+
+	case BDE_BTS_INSN_SIZE:
+	  return _("unknown instruction");
+
+	default:
+	  break;
+      }
+      break;
+
+#if defined (HAVE_LIBIPT)
+    case BTRACE_FORMAT_PT:
+      switch (errcode)
+      {
+	case BDE_PT_USER_QUIT:
+	  return _("trace decode cancelled");
+
+	case BDE_PT_DISABLED:
+	  return _("disabled");
+
+	case BDE_PT_OVERFLOW:
+	  return _("overflow");
+
+	default:
+	  if (errcode < 0)
+	    return pt_errstr (pt_errcode (errcode));
+	  break;
+      }
+      break;
+#endif /* defined (HAVE_LIBIPT)  */
+
+    default:
+      break;
+  }
+
+  return _("unknown");
+}
+
+/* See btrace.h.  */
+
 void
 btrace_fetch (struct thread_info *tp)
 {
diff --git a/gdb/btrace.h b/gdb/btrace.h
index 114242d..d0497b4 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -384,6 +384,11 @@ extern void btrace_disable (struct thread_info *);
    target_teardown_btrace instead of target_disable_btrace.  */
 extern void btrace_teardown (struct thread_info *);
 
+/* Return a human readable error string for the given ERRCODE in FORMAT.
+   The pointer will never be NULL and must not be freed.  */
+
+extern const char *btrace_decode_error (enum btrace_format format, int errcode);
+
 /* Fetch the branch trace for a single thread.  */
 extern void btrace_fetch (struct thread_info *);
 
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 6c50bab..0576123 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -464,63 +464,12 @@ static void
 btrace_ui_out_decode_error (struct ui_out *uiout, int errcode,
 			    enum btrace_format format)
 {
-  const char *errstr;
-  int is_error;
-
-  errstr = _("unknown");
-  is_error = 1;
-
-  switch (format)
-    {
-    default:
-      break;
-
-    case BTRACE_FORMAT_BTS:
-      switch (errcode)
-	{
-	default:
-	  break;
-
-	case BDE_BTS_OVERFLOW:
-	  errstr = _("instruction overflow");
-	  break;
-
-	case BDE_BTS_INSN_SIZE:
-	  errstr = _("unknown instruction");
-	  break;
-	}
-      break;
-
-#if defined (HAVE_LIBIPT)
-    case BTRACE_FORMAT_PT:
-      switch (errcode)
-	{
-	case BDE_PT_USER_QUIT:
-	  is_error = 0;
-	  errstr = _("trace decode cancelled");
-	  break;
-
-	case BDE_PT_DISABLED:
-	  is_error = 0;
-	  errstr = _("disabled");
-	  break;
-
-	case BDE_PT_OVERFLOW:
-	  is_error = 0;
-	  errstr = _("overflow");
-	  break;
-
-	default:
-	  if (errcode < 0)
-	    errstr = pt_errstr (pt_errcode (errcode));
-	  break;
-	}
-      break;
-#endif /* defined (HAVE_LIBIPT)  */
-    }
+  const char *errstr = btrace_decode_error (format, errcode);
 
   ui_out_text (uiout, _("["));
-  if (is_error)
+
+  /* ERRCODE > 0 indicates notifications on BTRACE_FORMAT_PT.  */
+  if (!(format == BTRACE_FORMAT_PT && errcode > 0))
     {
       ui_out_text (uiout, _("decode error ("));
       ui_out_field_int (uiout, "errcode", errcode);
-- 
2.7.4

RE: [PATCH v3 2/8] btrace: Export btrace_decode_error function.

[Metzger, Markus T]

> -----Original Message-----
> From: Wiederhake, Tim
> Sent: Monday, November 21, 2016 4:49 PM
> To: gdb-patches@sourceware.org
> Cc: palves@redhat.com; Metzger, Markus T <markus.t.metzger@intel.com>
> Subject: [PATCH v3 2/8] btrace: Export btrace_decode_error function.
> 
> 2016-11-21  Tim Wiederhake  <tim.wiederhake@intel.com>
> 
> gdb/ChangeLog:
> 
> 	* record-btrace.c (btrace_ui_out_decode_error): Move most of it ...
> 	* btrace.c (btrace_decode_error): ... here.  New function.
> 	* btrace.h (btrace_decode_error): New export.

> +const char *
> +btrace_decode_error (enum btrace_format format, int errcode)
> +{
> +  switch (format)
> +  {
> +    case BTRACE_FORMAT_BTS:

The { is indented and thus goes onto the same column than the case.


Looks good to me, otherwise.

Thanks,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928