gdb: output styling, and GDB code style fixes, for 'info frame'

I noticed that the output of 'info frame' was not styled much; the
frame's filename is styled, as are the names of any frame local
variables.  But there's lots of address and line number styling
missing.

When I started looking at the info_frame_command_core function though,
there were lots of coding standard changes that could be made too, so
while I was in the area I thought I'd make some coding standard
changes to the function:

  + Inlined local variable declarations.

  + Removed the 'caller_pc_p' local, and made 'caller_pc' a
    std::optional<CORE_ADDR> instead.

  + Converted a local from 'int' to 'bool'.

  + Replaced NULL with nullptr.

  + Replaced calls to gdb_printf with gdb_puts when the thing to print
    is a constant string.

  + Added { ... } around if/else bodies that are a single statement
    AND a comment inline with GDB coding style.

  + Avoid treating pointers and integers as booleans in 'if'
    conditions.  Compare to nullptr and/or zero instead.

Other than the new styling, there should be no user visible changes
after this commit.

Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/stack.c b/gdb/stack.c
index 2210a137586f5dd2b7da06a264d3347a98ce1bd4..9d8e9da6aa89377d8d2e1b1a39939304385cf617 100644 (file)
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1472,41 +1472,36 @@ frame_selection_by_function_completer (struct cmd_list_element *ignore,
 static void
 info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
 {
-  struct symbol *func;
-  struct symtab *s;
-  frame_info_ptr calling_frame_info;
-  int numregs;
-  const char *funname = 0;
-  enum language funlang = language_unknown;
-  const char *pc_regname;
-  struct gdbarch *gdbarch;
-  std::optional<CORE_ADDR> frame_pc;
-  /* Initialize it to avoid "may be used uninitialized" warning.  */
-  CORE_ADDR caller_pc = 0;
-  int caller_pc_p = 0;
-
-  gdbarch = get_frame_arch (fi);
+  struct gdbarch *gdbarch = get_frame_arch (fi);
 
   /* Name of the value returned by get_frame_pc().  Per comments, "pc"
      is not a good name.  */
+  const char *pc_regname;
   if (gdbarch_pc_regnum (gdbarch) >= 0)
-    /* OK, this is weird.  The gdbarch_pc_regnum hardware register's value can
-       easily not match that of the internal value returned by
-       get_frame_pc().  */
-    pc_regname = gdbarch_register_name (gdbarch, gdbarch_pc_regnum (gdbarch));
+    {
+      /* OK, this is weird.  The gdbarch_pc_regnum hardware register's
+        value can easily not match that of the internal value returned by
+        get_frame_pc().  */
+      pc_regname = gdbarch_register_name (gdbarch, gdbarch_pc_regnum (gdbarch));
+      gdb_assert (pc_regname != nullptr);
+    }
   else
-    /* But then, this is weird to.  Even without gdbarch_pc_regnum, an
-       architectures will often have a hardware register called "pc",
-       and that register's value, again, can easily not match
-       get_frame_pc().  */
-    pc_regname = "pc";
-
-  frame_pc = get_frame_pc_if_available (fi);
-  func = get_frame_function (fi);
+    {
+      /* But then, this is weird to.  Even without gdbarch_pc_regnum, an
+        architectures will often have a hardware register called "pc",
+        and that register's value, again, can easily not match
+        get_frame_pc().  */
+      pc_regname = "pc";
+    }
+
+  const char *funname = nullptr;
+  enum language funlang = language_unknown;
+  std::optional<CORE_ADDR> frame_pc = get_frame_pc_if_available (fi);
+  struct symbol *func = get_frame_function (fi);
   symtab_and_line sal = find_frame_sal (fi);
-  s = sal.symtab;
+  struct symtab *s = sal.symtab;
   gdb::unique_xmalloc_ptr<char> func_only;
-  if (func)
+  if (func != nullptr)
     {
       funname = func->print_name ();
       funlang = func->language ();
@@ -1519,20 +1514,20 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
             display parameters.  So remove the parameters.  */
          func_only = cp_remove_params (funname);
 
-         if (func_only)
+         if (func_only != nullptr)
            funname = func_only.get ();
        }
     }
   else if (frame_pc.has_value ())
     {
       bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (*frame_pc);
-      if (msymbol.minsym != NULL)
+      if (msymbol.minsym != nullptr)
        {
          funname = msymbol.minsym->print_name ();
          funlang = msymbol.minsym->language ();
        }
     }
-  calling_frame_info = get_prev_frame (fi);
+  frame_info_ptr calling_frame_info = get_prev_frame (fi);
 
   if (selected_frame_p && frame_relative_level (fi) >= 0)
     {
@@ -1541,41 +1536,43 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
     }
   else
     {
-      gdb_printf (_("Stack frame at "));
+      gdb_puts (_("Stack frame at "));
     }
-  gdb_puts (paddress (gdbarch, get_frame_base (fi)));
-  gdb_printf (":\n");
+  fputs_styled (paddress (gdbarch, get_frame_base (fi)),
+               address_style.style (), gdb_stdout);
+  gdb_puts (":\n");
   gdb_printf (" %s = ", pc_regname);
   if (frame_pc.has_value ())
-    gdb_puts (paddress (gdbarch, get_frame_pc (fi)));
+    fputs_styled (paddress (gdbarch, get_frame_pc (fi)),
+                 address_style.style (), gdb_stdout);
   else
     fputs_styled ("<unavailable>", metadata_style.style (), gdb_stdout);
 
   gdb_stdout->wrap_here (3);
-  if (funname)
+  if (funname != nullptr)
     {
-      gdb_printf (" in ");
-      gdb_puts (funname);
+      gdb_puts (" in ");
+      fputs_styled (funname, function_name_style.style (), gdb_stdout);
     }
   gdb_stdout->wrap_here (3);
-  if (sal.symtab)
+  if (sal.symtab != nullptr)
     gdb_printf
-      (" (%ps:%d)",
+      (" (%ps:%ps)",
        styled_string (file_name_style.style (),
                      symtab_to_filename_for_display (sal.symtab)),
-       sal.line);
+       styled_string (line_number_style.style (), pulongest (sal.line)));
   gdb_puts ("; ");
   gdb_stdout->wrap_here (4);
   gdb_printf ("saved %s = ", pc_regname);
 
+  std::optional<CORE_ADDR> caller_pc;
   if (!frame_id_p (frame_unwind_caller_id (fi)))
     val_print_not_saved (gdb_stdout);
   else
     {
       try
        {
-         caller_pc = frame_unwind_caller_pc (fi);
-         caller_pc_p = 1;
+         caller_pc.emplace (frame_unwind_caller_pc (fi));
        }
       catch (const gdb_exception_error &ex)
        {
@@ -1596,11 +1593,12 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
        }
     }
 
-  if (caller_pc_p)
-    gdb_puts (paddress (gdbarch, caller_pc));
-  gdb_printf ("\n");
+  if (caller_pc.has_value ())
+    fputs_styled (paddress (gdbarch, caller_pc.value ()),
+                 address_style.style (), gdb_stdout);
+  gdb_puts ("\n");
 
-  if (calling_frame_info == NULL)
+  if (calling_frame_info == nullptr)
     {
       enum unwind_stop_reason reason;
 
@@ -1616,21 +1614,23 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
                frame_relative_level (get_prev_frame (fi)));
   else
     {
-      gdb_printf (" called by frame at ");
-      gdb_puts (paddress (gdbarch, get_frame_base (calling_frame_info)));
+      gdb_puts (" called by frame at ");
+      fputs_styled (paddress (gdbarch, get_frame_base (calling_frame_info)),
+                   address_style.style (), gdb_stdout);
     }
-  if (get_next_frame (fi) && calling_frame_info)
+  if (get_next_frame (fi) != nullptr && calling_frame_info != nullptr)
     gdb_puts (",");
   gdb_stdout->wrap_here (3);
-  if (get_next_frame (fi))
+  if (get_next_frame (fi) != nullptr)
     {
-      gdb_printf (" caller of frame at ");
-      gdb_puts (paddress (gdbarch, get_frame_base (get_next_frame (fi))));
+      gdb_puts (" caller of frame at ");
+      fputs_styled (paddress (gdbarch, get_frame_base (get_next_frame (fi))),
+                   address_style.style (), gdb_stdout);
     }
-  if (get_next_frame (fi) || calling_frame_info)
+  if (get_next_frame (fi) != nullptr || calling_frame_info != nullptr)
     gdb_puts ("\n");
 
-  if (s)
+  if (s != nullptr)
     gdb_printf (" source language %s.\n",
                language_str (s->language ()));
 
@@ -1641,12 +1641,13 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
     int numargs;
 
     if (arg_list == 0)
-      gdb_printf (" Arglist at unknown address.\n");
+      gdb_puts (" Arglist at unknown address.\n");
     else
       {
-       gdb_printf (" Arglist at ");
-       gdb_puts (paddress (gdbarch, arg_list));
-       gdb_printf (",");
+       gdb_puts (" Arglist at ");
+       fputs_styled (paddress (gdbarch, arg_list), address_style.style (),
+                     gdb_stdout);
+       gdb_puts (",");
 
        if (!gdbarch_frame_num_args_p (gdbarch))
          {
@@ -1675,21 +1676,20 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
     CORE_ADDR arg_list = get_frame_locals_address (fi);
 
     if (arg_list == 0)
-      gdb_printf (" Locals at unknown address,");
+      gdb_puts (" Locals at unknown address,");
     else
       {
-       gdb_printf (" Locals at ");
-       gdb_puts (paddress (gdbarch, arg_list));
-       gdb_printf (",");
+       gdb_puts (" Locals at ");
+       fputs_styled (paddress (gdbarch, arg_list), address_style.style (),
+                     gdb_stdout);
+       gdb_puts (",");
       }
   }
 
   /* Print as much information as possible on the location of all the
      registers.  */
   {
-    int count;
-    int i;
-    int need_nl = 1;
+    bool need_nl = true;
     int sp_regnum = gdbarch_sp_regnum (gdbarch);
 
     /* The sp is special; what's displayed isn't the save address, but
@@ -1700,7 +1700,7 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
     if (sp_regnum >= 0)
       {
        struct value *value = frame_unwind_register_value (fi, sp_regnum);
-       gdb_assert (value != NULL);
+       gdb_assert (value != nullptr);
 
        if (!value->optimized_out () && value->entirely_available ())
          {
@@ -1713,29 +1713,31 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
                sp = extract_unsigned_integer
                  (value->contents_all ().data (), sp_size, byte_order);
 
-               gdb_printf (" Previous frame's sp is ");
-               gdb_puts (paddress (gdbarch, sp));
-               gdb_printf ("\n");
+               gdb_puts (" Previous frame's sp is ");
+               fputs_styled (paddress (gdbarch, sp), address_style.style (),
+                             gdb_stdout);
+               gdb_puts ("\n");
              }
            else if (value->lval () == lval_memory)
              {
-               gdb_printf (" Previous frame's sp at ");
-               gdb_puts (paddress (gdbarch, value->address ()));
-               gdb_printf ("\n");
+               gdb_puts (" Previous frame's sp at ");
+               fputs_styled (paddress (gdbarch, value->address ()),
+                             address_style.style (), gdb_stdout);
+               gdb_puts ("\n");
              }
            else if (value->lval () == lval_register)
              gdb_printf (" Previous frame's sp in %s\n",
                          gdbarch_register_name (gdbarch, value->regnum ()));
 
            release_value (value);
-           need_nl = 0;
+           need_nl = false;
          }
        /* else keep quiet.  */
       }
 
-    count = 0;
-    numregs = gdbarch_num_cooked_regs (gdbarch);
-    for (i = 0; i < numregs; i++)
+    int count = 0;
+    int numregs = gdbarch_num_cooked_regs (gdbarch);
+    for (int i = 0; i < numregs; i++)
       if (i != sp_regnum
          && gdbarch_register_reggroup_p (gdbarch, i, all_reggroup))
        {
@@ -1760,11 +1762,12 @@ info_frame_command_core (const frame_info_ptr &fi, bool selected_frame_p)
              gdb_stdout->wrap_here (1);
              gdb_printf (" %s at ",
                          gdbarch_register_name (gdbarch, i));
-             gdb_puts (paddress (gdbarch, addr));
+             fputs_styled (paddress (gdbarch, addr), address_style.style (),
+                           gdb_stdout);
              count++;
            }
        }
-    if (count || need_nl)
+    if (count > 0 || need_nl)
       gdb_puts ("\n");
   }
 }