From: Andrew Burgess Date: Fri, 9 Jan 2026 14:03:04 +0000 (+0000) Subject: gdb: output styling, and GDB code style fixes, for 'info frame' X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8c7c54a09d3a4a81761bc3b4fd88fb83a5f397ce;p=thirdparty%2Fbinutils-gdb.git 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 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 --- diff --git a/gdb/stack.c b/gdb/stack.c index 2210a137586..9d8e9da6aa8 100644 --- 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 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 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 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 ("", 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 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"); } }