[patch][python] 2 of 5 - Frame filter MI code changes.

Tom Tromey tromey@redhat.com
Wed Dec 5 17:11:00 GMT 2012


>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil>  #include "exceptions.h"
Phil> -
Phil> +#include "utils.h"
Phil> +#include "python/python.h"
Phil> +#include <ctype.h>
Phil>  enum what_to_list { locals, arguments, all };

Keep the blank line between includes and code.

Phil> +/* True if we want to allow Python-based frame filters.  */
Phil> +static int frame_filters = 0;
Phil> +
Phil> +void
Phil> +stack_enable_frame_filters (void)
Phil> +{
Phil> +  frame_filters = 1;
Phil> +}

I don't think you need this function, see below.

Phil> +static int
Phil> +parse_no_frames_option (char *arg)
Phil> +{
Phil> +  if (arg && (strcmp (arg, "--no-frame-filters") == 0))
Phil> +    return 1;
Phil> +
Phil> +  return 0;

I'd prefer it if the various callers were changed to use mi_getopt.
This provides uniformity and lets us add options later.

Phil> +  if (! raw_arg && frame_filters)
Phil> +    {
Phil> +      int count = frame_high;
Phil> +      int flags = PRINT_LEVEL | PRINT_FRAME_INFO;
Phil> +
Phil> +      if (frame_high != -1)
Phil> +	count = (frame_high - frame_low) + 1;
Phil> +
Phil> +      result = apply_frame_filter (fi, flags, 0, NULL, current_uiout,
Phil> +				   count);

I don't think I follow the high/low logic here.

How does this code strip off the first 'frame_low' frames?

Also, Do frame_low and frame_high refer to "raw" or "cooked" frames?
I tend to think they should refer to cooked ones, but I think at least
the answer should be explicit and documented.

This applies to the other functions as well.  I didn't read them as
closely due to this issue.

Phil> +  /* Run the inbuilt backtrace if there are no filters registered, or
Phil> +     if there was an error in the Python backtracing output, or if
Phil> +     frame-filters are disabled.  */

Discussing this on the other thread.

Phil> +  if (! frame_filters || raw_arg || result == PY_BT_ERROR
Phil> +      || result == PY_BT_NO_FILTERS)
Phil> +    {
Phil> +
Phil> +      /* Now let's print the frames up to frame_high, or until there are

Extra blank line.

Phil>  void
Phil> +mi_cmd_enable_frame_filters (char *command, char **argv, int argc)
Phil> +{
Phil> +  if (argc != 0)
Phil> +    error (_("-enable-frame-filters: no arguments allowed"));
Phil> +
Phil> +  stack_enable_frame_filters ();

I think just put this into mi-cmd-stack.c and remove
stack_enable_frame_filters.

Phil> +extern void
Phil> +stack_enable_frame_filters (void);

Zap this; but for future reference, put it all on one line.

Tom



More information about the Gdb-patches mailing list