public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Functions call history patches
@ 2022-04-25 14:48 Ari Hannula
  2022-04-25 14:48 ` [PATCH v2 1/2] [func_call] Add function-call-history-length command to MI Ari Hannula
  2022-04-25 14:48 ` [PATCH v2 2/2] [func_call] Add function-call-history " Ari Hannula
  0 siblings, 2 replies; 5+ messages in thread
From: Ari Hannula @ 2022-04-25 14:48 UTC (permalink / raw)
  To: gdb-patches

Hi,

I have reworked two of the patches that were posted in v1. The rest I have
decided to drop based on review comments. These patches are now only
related to Function Call History MI commands.

V1 of this series can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-February/185814.html

Changes since V1:
* Removed 3 patches from this patchset.
* Moved functions from record.c to new file mi/mi-cmd-record.c.
* Removed some unrelated changes
* Addressed review comments in tests

Regards,
Ari

Tim Wiederhake (2):
  [func_call] Add function-call-history-length command to MI.
  [func_call] Add function-call-history command to MI.

 gdb/Makefile.in                               |   1 +
 gdb/mi/mi-cmd-record.c                        |  83 ++++++++
 gdb/mi/mi-cmd-record.h                        |  32 +++
 gdb/mi/mi-cmds.c                              |   3 +
 gdb/mi/mi-cmds.h                              |   2 +
 gdb/record-btrace.c                           |  38 +++-
 gdb/record.c                                  |  28 +--
 gdb/record.h                                  |  18 ++
 gdb/target-delegates.c                        |  23 +++
 gdb/target.c                                  |   8 +
 gdb/target.h                                  |   7 +
 .../gdb.mi/mi-function_call_history.c         |  43 ++++
 .../gdb.mi/mi-function_call_history.exp       | 184 ++++++++++++++++++
 13 files changed, 452 insertions(+), 18 deletions(-)
 create mode 100644 gdb/mi/mi-cmd-record.c
 create mode 100644 gdb/mi/mi-cmd-record.h
 create mode 100644 gdb/testsuite/gdb.mi/mi-function_call_history.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-function_call_history.exp

-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] [func_call] Add function-call-history-length command to MI.
  2022-04-25 14:48 [PATCH v2 0/2] Functions call history patches Ari Hannula
@ 2022-04-25 14:48 ` Ari Hannula
  2022-04-26 18:24   ` Tom Tromey
  2022-04-25 14:48 ` [PATCH v2 2/2] [func_call] Add function-call-history " Ari Hannula
  1 sibling, 1 reply; 5+ messages in thread
From: Ari Hannula @ 2022-04-25 14:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tim Wiederhake, Ari Hannula

From: Tim Wiederhake <tim.wiederhake@intel.com>

This allows MI clients like Eclipse to retrieve the length of the function
call history.

gdb/ChangeLog:
2018-12-21  Tim Wiederhake  <tim.wiederhake@intel.com>
2022-04-25  Ari Hannula  <ari.hannula@intel.com>

	* mi/mi-cmds.c (mi_cmds): Add
	mi_cmd_record_function_call_history_length.
	* mi/mi-cmds.h (mi_cmd_record_function_call_history_length): New
	declaration.
	* record-btrace.c (record_btrace_target) <call_history_length>:
	New declaration.
	(record_btrace_target::call_history_length): New function.
	* mi/mi-cmd-record.c: New file.
	* mi/mi-cmd-record.h: New file.
	* Makefile.in (mi/mi-cmd-record.c): New definition.
	* target-delegates.c: Regenerated.
	* target.c (target_call_history_length): New function.
	* target.h (target_ops) <call_history_length>: New declaration.
	(target_call_history_length): New declaration.

gdb/testsuite/ChangeLog:
2018-12-21  Tim Wiederhake  <tim.wiederhake@intel.com>
2022-04-25  Ari Hannula  <ari.hannula@intel.com>

	* gdb.mi/mi-function_call_history.c: New file.
	* gdb.mi/mi-function_call_history.exp: New file.

Signed-off-by: Tim Wiederhake <tim.wiederhake@intel.com>
Signed-off-by: Ari Hannula <ari.hannula@intel.com>
---
 gdb/Makefile.in                               |  1 +
 gdb/mi/mi-cmd-record.c                        | 32 +++++++
 gdb/mi/mi-cmd-record.h                        | 27 ++++++
 gdb/mi/mi-cmds.c                              |  2 +
 gdb/mi/mi-cmds.h                              |  1 +
 gdb/record-btrace.c                           | 24 +++++
 gdb/target-delegates.c                        | 23 +++++
 gdb/target.c                                  |  8 ++
 gdb/target.h                                  |  7 ++
 .../gdb.mi/mi-function_call_history.c         | 43 +++++++++
 .../gdb.mi/mi-function_call_history.exp       | 89 +++++++++++++++++++
 11 files changed, 257 insertions(+)
 create mode 100644 gdb/mi/mi-cmd-record.c
 create mode 100644 gdb/mi/mi-cmd-record.h
 create mode 100644 gdb/testsuite/gdb.mi/mi-function_call_history.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-function_call_history.exp

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ec0e55dd80..c1de627fc2 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -283,6 +283,7 @@ SUBDIR_MI_SRCS = \
 	mi/mi-cmd-env.c \
 	mi/mi-cmd-file.c \
 	mi/mi-cmd-info.c \
+	mi/mi-cmd-record.c \
 	mi/mi-cmd-stack.c \
 	mi/mi-cmd-target.c \
 	mi/mi-cmd-var.c \
diff --git a/gdb/mi/mi-cmd-record.c b/gdb/mi/mi-cmd-record.c
new file mode 100644
index 0000000000..63af64aa03
--- /dev/null
+++ b/gdb/mi/mi-cmd-record.c
@@ -0,0 +1,32 @@
+/* Copyright (C) 2022 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+
+#include "defs.h"
+#include "target.h"
+#include "mi-cmd-record.h"
+
+void
+mi_cmd_record_function_call_history_length (const char *command,
+					    char **argv,
+					    int argc)
+{
+  if (argc != 0)
+    error (_("-function-call-history-length: Invalid number of arguments."));
+
+  target_call_history_length ();
+}
diff --git a/gdb/mi/mi-cmd-record.h b/gdb/mi/mi-cmd-record.h
new file mode 100644
index 0000000000..fbd5052dba
--- /dev/null
+++ b/gdb/mi/mi-cmd-record.h
@@ -0,0 +1,27 @@
+/* Copyright (C) 2022 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef MI_MI_CMD_RECORD_H
+#define MI_MI_CMD_RECORD_H
+
+/* The MI version of the command to get the length of the function call
+   history for record targets.  */
+extern void mi_cmd_record_function_call_history_length (const char *,
+							char **,
+							int);
+
+#endif /* MI_MI_CMD_RECORD_H */
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 0571469302..45a5692ff5 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -284,6 +284,8 @@ add_builtin_mi_commands ()
   add_mi_cmd_cli ("file-symbol-file", "symbol-file", 1);
   add_mi_cmd_mi ("fix-multi-location-breakpoint-output",
 		 mi_cmd_fix_multi_location_breakpoint_output),
+  add_mi_cmd_mi ("function-call-history-length",
+		 mi_cmd_record_function_call_history_length);
   add_mi_cmd_mi ("gdb-exit", mi_cmd_gdb_exit);
   add_mi_cmd_cli ("gdb-set", "set", 1,
 		  &mi_suppress_notification.cmd_param_changed);
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index 9ffb11bf99..ba990cf871 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -90,6 +90,7 @@ extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
 extern mi_cmd_argv_ftype mi_cmd_list_features;
 extern mi_cmd_argv_ftype mi_cmd_list_target_features;
 extern mi_cmd_argv_ftype mi_cmd_list_thread_groups;
+extern mi_cmd_argv_ftype mi_cmd_record_function_call_history_length;
 extern mi_cmd_argv_ftype mi_cmd_remove_inferior;
 extern mi_cmd_argv_ftype mi_cmd_stack_info_depth;
 extern mi_cmd_argv_ftype mi_cmd_stack_info_frame;
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 373d82b8b9..778a4ade0f 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -85,6 +85,7 @@ class record_btrace_target final : public target_ops
 			  gdb_disassembly_flags flags) override;
   void insn_history_range (ULONGEST begin, ULONGEST end,
 			   gdb_disassembly_flags flags) override;
+  void call_history_length () override;
   void call_history (int size, record_print_flags flags) override;
   void call_history_from (ULONGEST begin, int size, record_print_flags flags)
     override;
@@ -1217,6 +1218,29 @@ btrace_call_history (struct ui_out *uiout,
     }
 }
 
+/* The call_history_length method of target record-btrace.  */
+
+void
+record_btrace_target::call_history_length ()
+{
+  struct btrace_call_iterator end;
+  btrace_call_end (&end, require_btrace ());
+  btrace_call_prev (&end, 1);
+  const int length = btrace_call_number (&end);
+
+  if (current_uiout->is_mi_like_p ())
+    {
+      ui_out_emit_list list_emitter (current_uiout, "func history length");
+      ui_out_emit_tuple tuple_emitter (current_uiout, nullptr);
+      current_uiout->field_unsigned ("end", length);
+    }
+  else
+    {
+      ui_out_emit_tuple tuple_emitter (current_uiout, "func history length");
+      current_uiout->field_unsigned ("end", length);
+    }
+}
+
 /* The call_history method of target record-btrace.  */
 
 void
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 4e653e8f42..189036f264 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -185,6 +185,7 @@ struct dummy_target : public target_ops
   void insn_history (int arg0, gdb_disassembly_flags arg1) override;
   void insn_history_from (ULONGEST arg0, int arg1, gdb_disassembly_flags arg2) override;
   void insn_history_range (ULONGEST arg0, ULONGEST arg1, gdb_disassembly_flags arg2) override;
+  void call_history_length () override;
   void call_history (int arg0, record_print_flags arg1) override;
   void call_history_from (ULONGEST arg0, int arg1, record_print_flags arg2) override;
   void call_history_range (ULONGEST arg0, ULONGEST arg1, record_print_flags arg2) override;
@@ -359,6 +360,7 @@ struct debug_target : public target_ops
   void insn_history (int arg0, gdb_disassembly_flags arg1) override;
   void insn_history_from (ULONGEST arg0, int arg1, gdb_disassembly_flags arg2) override;
   void insn_history_range (ULONGEST arg0, ULONGEST arg1, gdb_disassembly_flags arg2) override;
+  void call_history_length () override;
   void call_history (int arg0, record_print_flags arg1) override;
   void call_history_from (ULONGEST arg0, int arg1, record_print_flags arg2) override;
   void call_history_range (ULONGEST arg0, ULONGEST arg1, record_print_flags arg2) override;
@@ -4256,6 +4258,27 @@ debug_target::insn_history_range (ULONGEST arg0, ULONGEST arg1, gdb_disassembly_
   gdb_puts (")\n", gdb_stdlog);
 }
 
+void
+target_ops::call_history_length ()
+{
+  this->beneath ()->call_history_length ();
+}
+
+void
+dummy_target::call_history_length ()
+{
+  tcomplain ();
+}
+
+void
+debug_target::call_history_length ()
+{
+  gdb_printf (gdb_stdlog, "-> %s->call_history_length (...)\n", this->beneath ()->shortname ());
+  this->beneath ()->call_history_length ();
+  gdb_printf (gdb_stdlog, "<- %s->call_history_length (", this->beneath ()->shortname ());
+  gdb_puts (")\n", gdb_stdlog);
+}
+
 void
 target_ops::call_history (int arg0, record_print_flags arg1)
 {
diff --git a/gdb/target.c b/gdb/target.c
index 7d291fd4d0..3cd1691e30 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -4217,6 +4217,14 @@ target_insn_history_range (ULONGEST begin, ULONGEST end,
 
 /* See target.h.  */
 
+void
+target_call_history_length ()
+{
+  current_inferior ()->top_target ()->call_history_length ();
+}
+
+/* See target.h.  */
+
 void
 target_call_history (int size, record_print_flags flags)
 {
diff --git a/gdb/target.h b/gdb/target.h
index 093178af0b..6cd7c465cc 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1249,6 +1249,10 @@ struct target_ops
 				     gdb_disassembly_flags flags)
       TARGET_DEFAULT_NORETURN (tcomplain ());
 
+    /* Print the number of functions in the recorded execution trace.  */
+    virtual void call_history_length ()
+      TARGET_DEFAULT_NORETURN (tcomplain ());
+
     /* Print a function trace of the recorded execution trace.
        If SIZE < 0, print abs (SIZE) preceding functions; otherwise, print SIZE
        succeeding functions.  */
@@ -2555,6 +2559,9 @@ extern void target_insn_history_from (ULONGEST from, int size,
 extern void target_insn_history_range (ULONGEST begin, ULONGEST end,
 				       gdb_disassembly_flags flags);
 
+/* See to_call_history_length.  */
+extern void target_call_history_length ();
+
 /* See to_call_history.  */
 extern void target_call_history (int size, record_print_flags flags);
 
diff --git a/gdb/testsuite/gdb.mi/mi-function_call_history.c b/gdb/testsuite/gdb.mi/mi-function_call_history.c
new file mode 100644
index 0000000000..3f82e21f8d
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-function_call_history.c
@@ -0,0 +1,43 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013-2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+inc (int i)
+{
+  return i + 1;
+}
+
+int
+fib (int n)
+{
+  if (n <= 1)
+    return n;
+
+  return fib (n - 2) + fib (n - 1);
+}
+
+int
+main ()
+{
+  int i, j;
+
+  for (i = 0; i < 10; i++)
+    j += inc (i);
+
+  j += fib (10); /* bp.1 */
+  return j; /* bp.2 */
+}
diff --git a/gdb/testsuite/gdb.mi/mi-function_call_history.exp b/gdb/testsuite/gdb.mi/mi-function_call_history.exp
new file mode 100644
index 0000000000..fcf5afeb35
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-function_call_history.exp
@@ -0,0 +1,89 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2013-2022 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test MI command function-call-history-length.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+if { [skip_btrace_tests] } {
+    untested mi-function_call_history.exp
+    return 0
+}
+
+gdb_exit
+if [mi_gdb_start separate-inferior-tty] {
+    continue
+}
+
+standard_testfile mi-function_call_history.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested "failed to compile"
+    return -1
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+if {[mi_runto_main] < 0} {
+    return -1
+}
+
+# start btrace
+send_gdb "record btrace\n"
+
+gdb_expect {
+    -re "=record-started,thread-group=\".*\"\r\n\\^done\r\n$mi_gdb_prompt$" {
+	# Done
+    }
+    -re "\\^error,msg=\"Target does not support branch tracing.\"" {
+	perror "Branch tracing not supported"
+	return -1
+    }
+    timeout {
+	perror "Unable to start branch tracing"
+	return -1
+    }
+}
+
+mi_create_breakpoint "-t $testfile.c:41" "insert temp breakpoint at $testfile.c:41" \
+    -number 2 -disp del -func main -file "$srcdir/$subdir/$testfile.c" -line 41
+
+mi_send_resuming_command "exec-continue" "continue to bp.1"
+
+mi_expect_stop "breakpoint-hit" "main" ".*" "$testfile.c" 41 \
+    {"" "disp=\".*\"" } "run to breakpoint bp.1"
+
+mi_gdb_test "125-function-call-history-length" \
+    "125\\^done,func history length=\\\[\{end=\"21\"\}\\\]" \
+    "125 function call history length"
+
+mi_create_breakpoint "-t $testfile.c:42" "insert temp breakpoint at $testfile.c:42" \
+    -number 3 -disp del -func main -file "$srcdir/$subdir/$testfile.c" -line 42
+
+mi_send_resuming_command "exec-continue" "continue to bp.2"
+
+mi_expect_stop "breakpoint-hit" "main" ".*" "$testfile.c" 42 \
+    {"" "disp=\".*\"" } "run to breakpoint bp.2"
+
+mi_gdb_test "225-function-call-history-length" \
+    "225\\^done,func history length=\\\[\{end=\"375\"\}\\\]" \
+    "225 function call history length"
+
+mi_gdb_exit
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] [func_call] Add function-call-history command to MI.
  2022-04-25 14:48 [PATCH v2 0/2] Functions call history patches Ari Hannula
  2022-04-25 14:48 ` [PATCH v2 1/2] [func_call] Add function-call-history-length command to MI Ari Hannula
@ 2022-04-25 14:48 ` Ari Hannula
  2022-04-26 18:32   ` Tom Tromey
  1 sibling, 1 reply; 5+ messages in thread
From: Ari Hannula @ 2022-04-25 14:48 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tim Wiederhake, Ari Hannula

From: Tim Wiederhake <tim.wiederhake@intel.com>

This allows MI clients like Eclipse to retrieve the function call history.

gdb/ChangeLog:
2018-12-27  Tim Wiederhake  <tim.wiederhake@intel.com>
2022-04-25  Ari Hannula  <ari.hannula@intel.com>

	* mi/mi-cmds.c (mi_cmds): Add mi_cmd_record_function_call_history.
	* mi/mi-cmds.h (mi_cmd_record_function_call_history): New
	declaration.
	* mi/mi-cmd-record.c (mi_cmd_record_function_call_history): New
	function.
	* mi/mi-cmd-record.h (mi_cmd_record_function_call_history): New
	declaration.
	* record.h: Externalize existing functions.
	* record.c: Externalize existing functions.
	* record-btrace.c: Fixed MI output formatting for call history.

gdb/testsuite/ChangeLog:
2018-12-27  Tim Wiederhake  <tim.wiederhake@intel.com>
2022-04-25  Ari Hannula  <ari.hannula@intel.com>

	* gdb.mi/mi-function_call_history.exp: Add tests.

Signed-off-by: Tim Wiederhake <tim.wiederhake@intel.com>
Signed-off-by: Ari Hannula <ari.hannula@intel.com>
---
 gdb/mi/mi-cmd-record.c                        | 53 ++++++++++-
 gdb/mi/mi-cmd-record.h                        |  5 +
 gdb/mi/mi-cmds.c                              |  1 +
 gdb/mi/mi-cmds.h                              |  1 +
 gdb/record-btrace.c                           | 14 ++-
 gdb/record.c                                  | 28 +++---
 gdb/record.h                                  | 18 ++++
 .../gdb.mi/mi-function_call_history.exp       | 95 +++++++++++++++++++
 8 files changed, 196 insertions(+), 19 deletions(-)

diff --git a/gdb/mi/mi-cmd-record.c b/gdb/mi/mi-cmd-record.c
index 63af64aa03..9d6a45baa2 100644
--- a/gdb/mi/mi-cmd-record.c
+++ b/gdb/mi/mi-cmd-record.c
@@ -15,9 +15,9 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-
 #include "defs.h"
 #include "target.h"
+#include "record.h"
 #include "mi-cmd-record.h"
 
 void
@@ -30,3 +30,54 @@ mi_cmd_record_function_call_history_length (const char *command,
 
   target_call_history_length ();
 }
+
+void
+mi_cmd_record_function_call_history (const char *command, char **argv,
+				     int argc)
+{
+  record_print_flags flags = 0;
+  unsigned int low, high;
+  char *charend;
+
+  if ((argc < 0) || (argc > 3))
+    error (_("-function-call-history: Invalid number of arguments."));
+
+  require_record_target ();
+
+  if ((argc == 1) || (argc == 3))
+    {
+      const char *targv = argv[0];
+      flags = get_call_history_modifiers (&targv);
+    }
+
+  if ((argc == 0) || (argc == 1))
+    {
+      const int size = command_size_to_target_size (
+	get_record_call_history_size ());
+      target_call_history (size, flags);
+      return;
+    }
+
+  if (argc == 3)
+    {
+      low = strtoul (argv[1], &charend, 10);
+      if (*charend != '\0')
+	error (_("Invalid syntax of begin func id '%s'"), argv[1]);
+
+      high = strtoul (argv[2], &charend, 10);
+      if (*charend != '\0')
+	error (_("Invalid syntax of end func id '%s'"), argv[2]);
+    }
+  else
+    {
+      low = strtoul (argv[0], &charend, 10);
+      if (*charend != '\0')
+	error (_("Invalid syntax of begin func id '%s'"), argv[0]);
+
+      high = strtoul (argv[1], &charend, 10);
+      if (*charend != '\0')
+	error (_("Invalid syntax of end func id '%s'"), argv[1]);
+    }
+
+  target_call_history_range (low, high, flags);
+}
diff --git a/gdb/mi/mi-cmd-record.h b/gdb/mi/mi-cmd-record.h
index fbd5052dba..da2f15fc8b 100644
--- a/gdb/mi/mi-cmd-record.h
+++ b/gdb/mi/mi-cmd-record.h
@@ -24,4 +24,9 @@ extern void mi_cmd_record_function_call_history_length (const char *,
 							char **,
 							int);
 
+/* The MI version of the command to get the function call history for
+   record targets.  */
+extern void mi_cmd_record_function_call_history (const char *, char **,
+						 int);
+
 #endif /* MI_MI_CMD_RECORD_H */
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 45a5692ff5..c40cdb5d6d 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -286,6 +286,7 @@ add_builtin_mi_commands ()
 		 mi_cmd_fix_multi_location_breakpoint_output),
   add_mi_cmd_mi ("function-call-history-length",
 		 mi_cmd_record_function_call_history_length);
+  add_mi_cmd_mi ("function-call-history", mi_cmd_record_function_call_history);
   add_mi_cmd_mi ("gdb-exit", mi_cmd_gdb_exit);
   add_mi_cmd_cli ("gdb-set", "set", 1,
 		  &mi_suppress_notification.cmd_param_changed);
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index ba990cf871..534d390813 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -91,6 +91,7 @@ extern mi_cmd_argv_ftype mi_cmd_list_features;
 extern mi_cmd_argv_ftype mi_cmd_list_target_features;
 extern mi_cmd_argv_ftype mi_cmd_list_thread_groups;
 extern mi_cmd_argv_ftype mi_cmd_record_function_call_history_length;
+extern mi_cmd_argv_ftype mi_cmd_record_function_call_history;
 extern mi_cmd_argv_ftype mi_cmd_remove_inferior;
 extern mi_cmd_argv_ftype mi_cmd_stack_info_depth;
 extern mi_cmd_argv_ftype mi_cmd_stack_info_frame;
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 778a4ade0f..b9fcb4c490 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1157,6 +1157,7 @@ btrace_call_history (struct ui_out *uiout,
 
   for (it = *begin; btrace_call_cmp (&it, end) < 0; btrace_call_next (&it, 1))
     {
+      ui_out_emit_tuple tuple_emitter (uiout, nullptr);
       const struct btrace_function *bfun;
       struct minimal_symbol *msym;
       struct symbol *sym;
@@ -1186,10 +1187,13 @@ btrace_call_history (struct ui_out *uiout,
 
       if ((flags & RECORD_PRINT_INDENT_CALLS) != 0)
 	{
-	  int level = bfun->level + btinfo->level, i;
+	  const int level = bfun->level + btinfo->level;
 
-	  for (i = 0; i < level; ++i)
-	    uiout->text ("  ");
+	  if (uiout->is_mi_like_p ())
+	    uiout->field_signed ("level", level);
+	  else
+	    for (int i = 0; i < level; ++i)
+	      uiout->text ("  ");
 	}
 
       if (sym != NULL)
@@ -1253,7 +1257,7 @@ record_btrace_target::call_history (int size, record_print_flags flags)
   unsigned int context, covered;
 
   uiout = current_uiout;
-  ui_out_emit_tuple tuple_emitter (uiout, "insn history");
+  ui_out_emit_list list_emitter (uiout, "func history");
   context = abs (size);
   if (context == 0)
     error (_("Bad record function-call-history-size."));
@@ -1340,7 +1344,7 @@ record_btrace_target::call_history_range (ULONGEST from, ULONGEST to,
   int found;
 
   uiout = current_uiout;
-  ui_out_emit_tuple tuple_emitter (uiout, "func history");
+  ui_out_emit_list list_emitter (uiout, "func history");
   low = from;
   high = to;
 
diff --git a/gdb/record.c b/gdb/record.c
index 17a5df262b..c4fed25f2f 100644
--- a/gdb/record.c
+++ b/gdb/record.c
@@ -65,10 +65,10 @@ find_record_target (void)
   return find_target_at (record_stratum);
 }
 
-/* Check that recording is active.  Throw an error, if it isn't.  */
+/* See record.h.  */
 
-static struct target_ops *
-require_record_target (void)
+target_ops *
+require_record_target ()
 {
   struct target_ops *t;
 
@@ -514,15 +514,9 @@ get_insn_history_modifiers (const char **arg)
   return modifiers;
 }
 
-/* The "set record instruction-history-size / set record
-   function-call-history-size" commands are unsigned, with UINT_MAX
-   meaning unlimited.  The target interfaces works with signed int
-   though, to indicate direction, so map "unlimited" to INT_MAX, which
-   is about the same as unlimited in practice.  If the user does have
-   a log that huge, she can fetch it in chunks across several requests,
-   but she'll likely have other problems first...  */
+/* See record.h.  */
 
-static int
+int
 command_size_to_target_size (unsigned int size)
 {
   gdb_assert (size <= INT_MAX || size == UINT_MAX);
@@ -596,9 +590,9 @@ cmd_record_insn_history (const char *arg, int from_tty)
     }
 }
 
-/* Read function-call-history modifiers from an argument string.  */
+/* See record.h.  */
 
-static record_print_flags
+record_print_flags
 get_call_history_modifiers (const char **arg)
 {
   record_print_flags modifiers = 0;
@@ -755,6 +749,14 @@ set_record_call_history_size (const char *args, int from_tty,
 			 &record_call_history_size);
 }
 
+/* See record.h.  */
+
+unsigned int
+get_record_call_history_size ()
+{
+  return record_call_history_size;
+}
+
 void _initialize_record ();
 void
 _initialize_record ()
diff --git a/gdb/record.h b/gdb/record.h
index 0fbca9d44c..b142ef22a4 100644
--- a/gdb/record.h
+++ b/gdb/record.h
@@ -102,6 +102,24 @@ extern void record_kill (struct target_ops *);
    Returns NULL if none is found.  */
 extern struct target_ops *find_record_target (void);
 
+/* Check that recording is active.  Throw an error, if it isn't.  */
+extern target_ops *require_record_target ();
+
+/* The "set record instruction-history-size / set record
+   function-call-history-size" commands are unsigned, with UINT_MAX
+   meaning unlimited.  The target interfaces works with signed int
+   though, to indicate direction, so map "unlimited" to INT_MAX, which
+   is about the same as unlimited in practice.  If the user does have
+   a log that huge, she can fetch it in chunks across several requests,
+   but she'll likely have other problems first...  */
+extern int command_size_to_target_size (unsigned int);
+
+/* Read function-call-history modifiers from an argument string.  */
+extern record_print_flags get_call_history_modifiers (const char **);
+
+/* Returns value of record_call_history_size.  */
+extern unsigned int get_record_call_history_size ();
+
 /* This is to be called by record_stratum targets' open routine before
    it does anything.  */
 extern void record_preopen (void);
diff --git a/gdb/testsuite/gdb.mi/mi-function_call_history.exp b/gdb/testsuite/gdb.mi/mi-function_call_history.exp
index fcf5afeb35..ca1ccb27f0 100644
--- a/gdb/testsuite/gdb.mi/mi-function_call_history.exp
+++ b/gdb/testsuite/gdb.mi/mi-function_call_history.exp
@@ -70,10 +70,57 @@ mi_send_resuming_command "exec-continue" "continue to bp.1"
 mi_expect_stop "breakpoint-hit" "main" ".*" "$testfile.c" 41 \
     {"" "disp=\".*\"" } "run to breakpoint bp.1"
 
+mi_gdb_test "121-function-call-history" \
+    "121\\^done,func history=\\\[\{index=\"12\",function=\"inc\"\},\{index=\"13\",function=\"main\"\},\{index=\"14\",function=\"inc\"\},\{index=\"15\",function=\"main\"\},\{index=\"16\",function=\"inc\"\},\{index=\"17\",function=\"main\"\},\{index=\"18\",function=\"inc\"\},\{index=\"19\",function=\"main\"\},\{index=\"20\",function=\"inc\"\},\{index=\"21\",function=\"main\"\}\\\]" \
+    "121 function call history default without flags"
+
+send_gdb "122-function-call-history\n"
+gdb_expect {
+    -re "\"At the end of the branch trace record..*\\^done,func history=[].*$mi_gdb_prompt$" {
+	#Done
+    }
+    default {
+	perror "122 Function call history failed"
+	return -1
+    }
+}
+
+mi_gdb_test "123-function-call-history 12 16" \
+    "123\\^done,func history=\\\[\{index=\"12\",function=\"inc\"\},\{index=\"13\",function=\"main\"\},\{index=\"14\",function=\"inc\"\},\{index=\"15\",function=\"main\"\},\{index=\"16\",function=\"inc\"\}\\\]" \
+    "123 function call history range without flags"
+
+mi_gdb_test "124-function-call-history 12 13" \
+    "124\\^done,func history=\\\[\{index=\"12\",function=\"inc\"\},\{index=\"13\",function=\"main\"\}\\\]" \
+    "124 function call history range without flags"
+
 mi_gdb_test "125-function-call-history-length" \
     "125\\^done,func history length=\\\[\{end=\"21\"\}\\\]" \
     "125 function call history length"
 
+mi_gdb_test "126-function-call-history 22 23" \
+    "126\\^error,msg=\"Range out of bounds.\"" \
+    "126 function call history range error check"
+
+mi_gdb_test "127-function-call-history /lc" \
+    "127\\^done,func history=\\\[\{index=\"14\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"15\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"16\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"17\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"18\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"19\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"20\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"21\",level=\"0\",function=\"main\",file=\"$srcdir/$subdir/$testfile.c\",.*\}\\\]" \
+    "127 function call history default with flags"
+
+mi_gdb_test "128-function-call-history /lc 14 20" \
+    "128\\^done,func history=\\\[\{index=\"14\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"15\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"16\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"17\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"18\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"19\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"20\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*\}\\\]" \
+    "128 function call history range with flags"
+
+mi_gdb_test "129-function-call-history /lc 1 5" \
+    "129\\^done,func history=\\\[\{index=\"1\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"2\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"3\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"4\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"5\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*\}\\\]" \
+    "129 function call history range with flags"
+
+mi_gdb_test "130-function-call-history /l 1 5" \
+    "130\\^done,func history=\\\[\{index=\"1\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"2\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"3\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"4\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"5\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*\}\\\]" \
+    "130 function call history range with flags"
+
+mi_gdb_test "131-function-call-history /c 1 5" \
+    "131\\^done,func history=\\\[\{index=\"1\",level=\"0\",function=\"main\"\},\{index=\"2\",level=\"1\",function=\"inc\"\},\{index=\"3\",level=\"0\",function=\"main\"\},\{index=\"4\",level=\"1\",function=\"inc\"\},\{index=\"5\",level=\"0\",function=\"main\"\}\\\]" \
+    "131 function call history range with flags"
+
 mi_create_breakpoint "-t $testfile.c:42" "insert temp breakpoint at $testfile.c:42" \
     -number 3 -disp del -func main -file "$srcdir/$subdir/$testfile.c" -line 42
 
@@ -82,8 +129,56 @@ mi_send_resuming_command "exec-continue" "continue to bp.2"
 mi_expect_stop "breakpoint-hit" "main" ".*" "$testfile.c" 42 \
     {"" "disp=\".*\"" } "run to breakpoint bp.2"
 
+mi_gdb_test "221-function-call-history" \
+    "221\\^done,func history=\\\[\{index=\"366\",function=\"fib\"\},\{index=\"367\",function=\"fib\"\},\{index=\"368\",function=\"fib\"\},\{index=\"369\",function=\"fib\"\},\{index=\"370\",function=\"fib\"\},\{index=\"371\",function=\"fib\"\},\{index=\"372\",function=\"fib\"\},\{index=\"373\",function=\"fib\"\},\{index=\"374\",function=\"fib\"\},\{index=\"375\",function=\"main\"\}\\\]" \
+    "221 function call history default without flags"
+
+send_gdb "222-function-call-history\n"
+
+gdb_expect {
+    -re "\"At the end of the branch trace record..*\\^done,func history=[].*$mi_gdb_prompt$" {
+	#Done
+    }
+    default {
+	perror "222 Function call history failed"
+	return -1
+    }
+}
+
+mi_gdb_test "223-function-call-history 12 16" \
+    "223\\^done,func history=\\\[\{index=\"12\",function=\"inc\"\},\{index=\"13\",function=\"main\"\},\{index=\"14\",function=\"inc\"\},\{index=\"15\",function=\"main\"\},\{index=\"16\",function=\"inc\"\}\\\]" \
+    "223 function call history range without flags"
+
+mi_gdb_test "224-function-call-history 366 368" \
+    "224\\^done,func history=\\\[\{index=\"366\",function=\"fib\"\},\{index=\"367\",function=\"fib\"\},\{index=\"368\",function=\"fib\"\}\\\]" \
+    "224 function call history range without flags"
+
 mi_gdb_test "225-function-call-history-length" \
     "225\\^done,func history length=\\\[\{end=\"375\"\}\\\]" \
     "225 function call history length"
 
+mi_gdb_test "226-function-call-history 376 400" \
+    "226\\^error,msg=\"Range out of bounds.\"" \
+    "226 function call history range error check"
+
+mi_gdb_test "227-function-call-history /lc" \
+    "227\\^done,func history=\\\[\{index=\"369\",level=\"6\",function=\"fib\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"370\",level=\"5\",function=\"fib\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"371\",level=\"4\",function=\"fib\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"372\",level=\"3\",function=\"fib\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"373\",level=\"2\",function=\"fib\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"374\",level=\"1\",function=\"fib\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"375\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*}\\\]" \
+    "227 function call history default with flags"
+
+mi_gdb_test "228-function-call-history /lc 14 20" \
+    "228\\^done,func history=\\\[\{index=\"14\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"15\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"16\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"17\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"18\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"19\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"20\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*\}\\\]" \
+    "228 function call history range with flags"
+
+mi_gdb_test "229-function-call-history /lc 1 5" \
+    "229\\^done,func history=\\\[\{index=\"1\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"2\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"3\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"4\",level=\"1\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"5\",level=\"0\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*\}\\\]" \
+    "229 function call history range with flags"
+
+mi_gdb_test "230-function-call-history /l 1 5" \
+    "230\\^done,func history=\\\[\{index=\"1\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"2\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"3\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"4\",function=\"inc\"\,file=\"$srcdir/$subdir/$testfile.c\",.*},\{index=\"5\",function=\"main\"\,file=\"$srcdir/$subdir/$testfile.c\",.*\}\\\]" \
+    "230 function call history range with flags"
+
+mi_gdb_test "231-function-call-history /c 1 5" \
+    "231\\^done,func history=\\\[\{index=\"1\",level=\"0\",function=\"main\"\},\{index=\"2\",level=\"1\",function=\"inc\"\},\{index=\"3\",level=\"0\",function=\"main\"\},\{index=\"4\",level=\"1\",function=\"inc\"\},\{index=\"5\",level=\"0\",function=\"main\"\}\\\]" \
+    "231 function call history range with flags"
+
 mi_gdb_exit
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] [func_call] Add function-call-history-length command to MI.
  2022-04-25 14:48 ` [PATCH v2 1/2] [func_call] Add function-call-history-length command to MI Ari Hannula
@ 2022-04-26 18:24   ` Tom Tromey
  0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2022-04-26 18:24 UTC (permalink / raw)
  To: Ari Hannula via Gdb-patches; +Cc: Ari Hannula, Tim Wiederhake

>>>>> "Ari" == Ari Hannula via Gdb-patches <gdb-patches@sourceware.org> writes:

Ari> From: Tim Wiederhake <tim.wiederhake@intel.com>
Ari> This allows MI clients like Eclipse to retrieve the length of the function
Ari> call history.

Ari> gdb/ChangeLog:
Ari> 2018-12-21  Tim Wiederhake  <tim.wiederhake@intel.com>
Ari> 2022-04-25  Ari Hannula  <ari.hannula@intel.com>

We don't use ChangeLog any more, so you can just remove this part of the
commit message.

Ari> +  if (current_uiout->is_mi_like_p ())
Ari> +    {
Ari> +      ui_out_emit_list list_emitter (current_uiout, "func history length");
Ari> +      ui_out_emit_tuple tuple_emitter (current_uiout, nullptr);
Ari> +      current_uiout->field_unsigned ("end", length);
Ari> +    }
Ari> +  else
Ari> +    {
Ari> +      ui_out_emit_tuple tuple_emitter (current_uiout, "func history length");
Ari> +      current_uiout->field_unsigned ("end", length);
Ari> +    }

Do we have other cases where an MI field name includes spaces?  It seems
a little weird to me.

Why does this code have to check is_mi_like_p?  I think it would be
better not to introduce new uses of this if possible.

Also I'm wondering why this API emits the output itself rather than just
returning some value and letting the caller do the printing.

Finally, all MI command additions should come with documentation.

thanks,
Tom

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] [func_call] Add function-call-history command to MI.
  2022-04-25 14:48 ` [PATCH v2 2/2] [func_call] Add function-call-history " Ari Hannula
@ 2022-04-26 18:32   ` Tom Tromey
  0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2022-04-26 18:32 UTC (permalink / raw)
  To: Ari Hannula via Gdb-patches; +Cc: Ari Hannula, Tim Wiederhake

>>>>> "Ari" == Ari Hannula via Gdb-patches <gdb-patches@sourceware.org> writes:

Ari> +  if ((argc < 0) || (argc > 3))

All these 'if's are over-parenthesized.

Ari> +      const int size = command_size_to_target_size (
Ari> +	get_record_call_history_size ());

The formatting here looks weird.  If it goes past the line length the
gdb style would be to break before the '=', not after '('.

Ari> -  ui_out_emit_tuple tuple_emitter (uiout, "insn history");
Ari> +  ui_out_emit_list list_emitter (uiout, "func history");

Doesn't this change the MI output in an incompatible way?

Ari> -  ui_out_emit_tuple tuple_emitter (uiout, "func history");
Ari> +  ui_out_emit_list list_emitter (uiout, "func history");

Here too.

This patch also needs documentation.

thanks,
Tom

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-04-26 18:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-25 14:48 [PATCH v2 0/2] Functions call history patches Ari Hannula
2022-04-25 14:48 ` [PATCH v2 1/2] [func_call] Add function-call-history-length command to MI Ari Hannula
2022-04-26 18:24   ` Tom Tromey
2022-04-25 14:48 ` [PATCH v2 2/2] [func_call] Add function-call-history " Ari Hannula
2022-04-26 18:32   ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).