[PATCHv2,1/2] gdb: Split func_command into two parts.
Commit Message
Thanks for the feedback.
* Pedro Alves <palves@redhat.com> [2018-05-18 20:38:26 +0100]:
> On 05/08/2018 05:58 PM, Andrew Burgess wrote:
> > The func_command function is used to emulate the dbx 'func' command.
> > However, finding a stack frame based on function name might be a useful
> > feature, and so the core of func_command is now split out into a
> > separate function.
> >
> > gdb/ChangeLog:
> >
> > * stack.c (select_and_print_frame): Delete.
> > (func_command): Most content moved into new function
> > find_frame_for_function, use new function, print result, add
> > function comment.
> > (find_frame_for_function): New function, now returns a result.
>
> This LGTM, with a couple minor nits.
>
> > /* Return the symbol-block in which the selected frame is executing.
> > Can return zero under various legitimate circumstances.
> > @@ -2460,19 +2450,19 @@ struct function_bounds
> > CORE_ADDR low, high;
> > };
> >
> > -static void
> > -func_command (const char *arg, int from_tty)
> > +static struct frame_info *
> > +find_frame_for_function (const char *function_name)
>
> There's a comment above the structure that is describing
> what the func_command function did. That needs to be
> updated to describe what the new function does.
As the structure was so small, and only used in that one function, I
moved the structure into the function entirely (and added an updated
comment to both the structure, and the function). I had a little look
through GDB and there are a few other cases of structures declared
within a function, so hopefully this is OK. Let me know if you'd
rather see this moved back out again.
>
> > +
> > +/* Implements the dbx 'func' command. */
> > +
> > +static void
> > +func_command (const char *arg, int from_tty)
> > +{
> > + struct frame_info *frame;
> > +
> > + if (arg == NULL)
> > + return;
> > +
> > + frame = find_frame_for_function (arg);
>
> You can declare and initialize at the same time:
Done.
---
gdb: Split func_command into two parts.
The func_command function is used to emulate the dbx 'func' command.
However, finding a stack frame based on function name might be a useful
feature, and so the core of func_command is now split out into a
separate function.
gdb/ChangeLog:
* stack.c (select_and_print_frame): Delete.
(struct function_bounds): Move struct within function.
(func_command): Most content moved into new function
find_frame_for_function, use new function, print result, add
function comment.
(find_frame_for_function): New function, now returns a result.
---
gdb/ChangeLog | 9 ++++++++
gdb/stack.c | 67 +++++++++++++++++++++++++++++++++--------------------------
2 files changed, 46 insertions(+), 30 deletions(-)
Comments
On 05/21/2018 04:27 PM, Andrew Burgess wrote:
> As the structure was so small, and only used in that one function, I
> moved the structure into the function entirely (and added an updated
> comment to both the structure, and the function). I had a little look
> through GDB and there are a few other cases of structures declared
> within a function, so hopefully this is OK. Let me know if you'd
> rather see this moved back out again.
This is fine with me as is. Feel free to push it in immediately
to get it out of the way.
Thanks,
Pedro Alves
@@ -2156,16 +2156,6 @@ info_args_command (const char *ignore, int from_tty)
print_frame_arg_vars (get_selected_frame (_("No frame selected.")),
gdb_stdout);
}
-
-/* Select frame FRAME. Also print the stack frame and show the source
- if this is the tui version. */
-static void
-select_and_print_frame (struct frame_info *frame)
-{
- select_frame (frame);
- if (frame)
- print_stack_frame (frame, 1, SRC_AND_LOC, 1);
-}
/* Return the symbol-block in which the selected frame is executing.
Can return zero under various legitimate circumstances.
@@ -2451,29 +2441,30 @@ return_command (const char *retval_exp, int from_tty)
print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
}
-/* Sets the scope to input function name, provided that the function
- is within the current stack frame. */
-
-struct function_bounds
-{
- CORE_ADDR low, high;
-};
+/* Find the most inner frame in the current stack for a function called
+ FUNCTION_NAME. If no matching frame is found return NULL. */
-static void
-func_command (const char *arg, int from_tty)
+static struct frame_info *
+find_frame_for_function (const char *function_name)
{
+ /* Used to hold the lower and upper addresses for each of the
+ SYMTAB_AND_LINEs found for functions matching FUNCTION_NAME. */
+ struct function_bounds
+ {
+ CORE_ADDR low, high;
+ };
struct frame_info *frame;
- int found = 0;
+ bool found = false;
int level = 1;
- if (arg == NULL)
- return;
+ gdb_assert (function_name != NULL);
frame = get_current_frame ();
std::vector<symtab_and_line> sals
- = decode_line_with_current_source (arg, DECODE_LINE_FUNFIRSTLINE);
+ = decode_line_with_current_source (function_name,
+ DECODE_LINE_FUNFIRSTLINE);
gdb::def_vector<function_bounds> func_bounds (sals.size ());
- for (size_t i = 0; (i < sals.size () && !found); i++)
+ for (size_t i = 0; i < sals.size (); i++)
{
if (sals[i].pspace != current_program_space)
func_bounds[i].low = func_bounds[i].high = 0;
@@ -2481,9 +2472,7 @@ func_command (const char *arg, int from_tty)
|| find_pc_partial_function (sals[i].pc, NULL,
&func_bounds[i].low,
&func_bounds[i].high) == 0)
- {
- func_bounds[i].low = func_bounds[i].high = 0;
- }
+ func_bounds[i].low = func_bounds[i].high = 0;
}
do
@@ -2500,9 +2489,27 @@ func_command (const char *arg, int from_tty)
while (!found && level == 0);
if (!found)
- printf_filtered (_("'%s' not within current stack frame.\n"), arg);
- else if (frame != get_selected_frame (NULL))
- select_and_print_frame (frame);
+ frame = NULL;
+
+ return frame;
+}
+
+/* Implements the dbx 'func' command. */
+
+static void
+func_command (const char *arg, int from_tty)
+{
+ if (arg == NULL)
+ return;
+
+ struct frame_info *frame = find_frame_for_function (arg);
+ if (frame == NULL)
+ error (_("'%s' not within current stack frame.\n"), arg);
+ if (frame != get_selected_frame (NULL))
+ {
+ select_frame (frame);
+ print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC, 1);
+ }
}
void