This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA v2 01/24] Introduce and use ui_out_emit_table


Hi Tom,

Just some formatting comments, I think it's fine to push with those fixed.

@@ -6851,48 +6850,43 @@ breakpoint_1 (char *args, int allflag,
 	}
     }

-  if (opts.addressprint)
-    bkpttbl_chain
-      = make_cleanup_ui_out_table_begin_end (uiout, 6,
-					     nr_printable_breakpoints,
-                                             "BreakpointTable");
-  else
-    bkpttbl_chain
-      = make_cleanup_ui_out_table_begin_end (uiout, 5,
-					     nr_printable_breakpoints,
-                                             "BreakpointTable");
-
-  if (nr_printable_breakpoints > 0)
-    annotate_breakpoints_headers ();
-  if (nr_printable_breakpoints > 0)
-    annotate_field (0);
-  uiout->table_header (7, ui_left, "number", "Num"); /* 1 */
-  if (nr_printable_breakpoints > 0)
-    annotate_field (1);
- uiout->table_header (print_type_col_width, ui_left, "type", "Type"); /* 2 */
-  if (nr_printable_breakpoints > 0)
-    annotate_field (2);
-  uiout->table_header (4, ui_left, "disp", "Disp"); /* 3 */
-  if (nr_printable_breakpoints > 0)
-    annotate_field (3);
-  uiout->table_header (3, ui_left, "enabled", "Enb"); /* 4 */
-  if (opts.addressprint)
-    {
-      if (nr_printable_breakpoints > 0)
-	annotate_field (4);
-      if (print_address_bits <= 32)
-	uiout->table_header (10, ui_left, "addr", "Address"); /* 5 */
-      else
-	uiout->table_header (18, ui_left, "addr", "Address"); /* 5 */
-    }
-  if (nr_printable_breakpoints > 0)
-    annotate_field (5);
-  uiout->table_header (40, ui_noalign, "what", "What"); /* 6 */
-  uiout->table_body ();
-  if (nr_printable_breakpoints > 0)
-    annotate_breakpoints_table ();
+  {
+    ui_out_emit_table table_emitter (uiout,
+				     opts.addressprint ? 6 : 5,
+				     nr_printable_breakpoints,
+				     "BreakpointTable");
+
+    if (nr_printable_breakpoints > 0)
+      annotate_breakpoints_headers ();
+    if (nr_printable_breakpoints > 0)
+      annotate_field (0);
+    uiout->table_header (7, ui_left, "number", "Num"); /* 1 */
+    if (nr_printable_breakpoints > 0)
+      annotate_field (1);
+    uiout->table_header (print_type_col_width, ui_left, "type",
"Type"); /* 2 */
+    if (nr_printable_breakpoints > 0)
+      annotate_field (2);
+    uiout->table_header (4, ui_left, "disp", "Disp"); /* 3 */
+    if (nr_printable_breakpoints > 0)
+      annotate_field (3);
+    uiout->table_header (3, ui_left, "enabled", "Enb"); /* 4 */
+    if (opts.addressprint)
+      {
+	if (nr_printable_breakpoints > 0)
+	  annotate_field (4);
+	if (print_address_bits <= 32)
+	  uiout->table_header (10, ui_left, "addr", "Address"); /* 5 */
+	else
+	  uiout->table_header (18, ui_left, "addr", "Address"); /* 5 */
+      }
+    if (nr_printable_breakpoints > 0)
+      annotate_field (5);
+    uiout->table_header (40, ui_noalign, "what", "What"); /* 6 */
+    uiout->table_body ();
+    if (nr_printable_breakpoints > 0)
+      annotate_breakpoints_table ();

-  ALL_BREAKPOINTS (b)
+    ALL_BREAKPOINTS (b)

Shouldn't the scope just under this be indented as well?

@@ -1075,19 +1074,18 @@ info_sharedlibrary_command (char *pattern, int from_tty)
 	}
     }

-  table_cleanup =
-    make_cleanup_ui_out_table_begin_end (uiout, 4, nr_libs,
-					 "SharedLibraryTable");
+  {
+ ui_out_emit_table table_emitter (uiout, 4, nr_libs, "SharedLibraryTable");

-  /* The "- 1" is because ui_out adds one space between columns.  */
-  uiout->table_header (addr_width - 1, ui_left, "from", "From");
-  uiout->table_header (addr_width - 1, ui_left, "to", "To");
-  uiout->table_header (12 - 1, ui_left, "syms-read", "Syms Read");
- uiout->table_header (0, ui_noalign, "name", "Shared Object Library");
+    /* The "- 1" is because ui_out adds one space between columns.  */
+    uiout->table_header (addr_width - 1, ui_left, "from", "From");
+    uiout->table_header (addr_width - 1, ui_left, "to", "To");
+    uiout->table_header (12 - 1, ui_left, "syms-read", "Syms Read");
+ uiout->table_header (0, ui_noalign, "name", "Shared Object Library");

-  uiout->table_body ();
+    uiout->table_body ();

-  ALL_SO_LIBS (so)
+    ALL_SO_LIBS (so)

I think the scope below should be indented as well.

     {
       if (! so->so_name[0])
 	continue;
@@ -1121,8 +1119,7 @@ info_sharedlibrary_command (char *pattern, int from_tty)

       uiout->text ("\n");
     }
-
-  do_cleanups (table_cleanup);
+  }

   if (nr_libs == 0)
     {
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 4f2bac5..6721e22 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -482,7 +482,6 @@ tvariables_info_1 (void)
   struct trace_state_variable *tsv;
   int ix;
   int count = 0;
-  struct cleanup *back_to;
   struct ui_out *uiout = current_uiout;

   if (VEC_length (tsv_s, tvariables) == 0 && !uiout->is_mi_like_p ())
@@ -496,8 +495,7 @@ tvariables_info_1 (void)
tsv->value_known = target_get_trace_state_variable_value (tsv->number,
 							      &(tsv->value));

-  back_to = make_cleanup_ui_out_table_begin_end (uiout, 3,
- count, "trace-variables"); + ui_out_emit_table table_emitter (uiout, 3, count, "trace-variables");
   uiout->table_header (15, ui_left, "name", "Name");
   uiout->table_header (11, ui_left, "initial", "Initial");
   uiout->table_header (11, ui_left, "current", "Current");
@@ -531,8 +529,6 @@ tvariables_info_1 (void)
         uiout->field_string ("current", c);
       uiout->text ("\n");
     }
-
-  do_cleanups (back_to);
 }

 /* List all the trace state variables.  */
@@ -3952,9 +3948,8 @@ info_static_tracepoint_markers_command (char
*arg, int from_tty)
don't work without in-process agent, so we don't bother users to type
      `set agent on' when to use static tracepoint.  */

-  old_chain
-    = make_cleanup_ui_out_table_begin_end (uiout, 5, -1,
-					   "StaticTracepointMarkersTable");
+  ui_out_emit_table table_emitter (uiout, 5, -1,
+				   "StaticTracepointMarkersTable");

   uiout->table_header (7, ui_left, "counter", "Cnt");

@@ -3970,7 +3965,7 @@ info_static_tracepoint_markers_command (char
*arg, int from_tty)
   uiout->table_body ();

   markers = target_static_tracepoint_markers_by_strid (NULL);
-  make_cleanup (VEC_cleanup (static_tracepoint_marker_p), &markers);
+  old_chain = make_cleanup (VEC_cleanup (static_tracepoint_marker_p),
&markers);

   for (i = 0;
        VEC_iterate (static_tracepoint_marker_p,
diff --git a/gdb/ui-out.h b/gdb/ui-out.h
index 9278cab..2293f26 100644
--- a/gdb/ui-out.h
+++ b/gdb/ui-out.h
@@ -220,4 +220,31 @@ private:
 typedef ui_out_emit_type<ui_out_type_tuple> ui_out_emit_tuple;
 typedef ui_out_emit_type<ui_out_type_list> ui_out_emit_list;

+/* This is similar to make_cleanup_ui_out_table_begin_end, but written
+   as an RAII class.  */
+class ui_out_emit_table
+{
+public:
+
+  ui_out_emit_table (struct ui_out *uiout, int nr_cols, int nr_rows,
+		     const char *tblid)
+

Spurious empty line?

+    : m_uiout (uiout)
+  {
+    m_uiout->table_begin (nr_cols, nr_rows, tblid);
+  }
+
+  ~ui_out_emit_table ()
+  {
+    m_uiout->table_end ();
+  }
+
+  ui_out_emit_table (const ui_out_emit_table &) = delete;
+  ui_out_emit_table &operator= (const ui_out_emit_table &) = delete;
+
+private:
+
+  struct ui_out *m_uiout;
+};
+
 #endif /* UI_OUT_H */

Thanks,

Simon


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]