Re: [patch] delete a range of display numbers

[Guillaume Leconte <guillaume.leconte@gmail.com>]

Alright the code works well, with the expected behavious.  I think
it's obviously the best solution, since it reuses existing code.

Thanks.

On Fri, Feb 18, 2011 at 12:58 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> The "delete" breakpoint command accepts ranges as
> well, and even has code that handles convenience
> variables mixed with the numbers.  I think we should
> reuse that instead of re-adding code that parses ranges.
> Might as well make the "delete display" command
> implementation look more like the "delete" command
> implementation.  Here's a quick cut at it.
>
> --
> Pedro Alves
>
> ---
>  gdb/breakpoint.c |    5 +---
>  gdb/breakpoint.h |    2 +
>  gdb/printcmd.c   |   62 ++++++++++++++++++++++++++++---------------------------
>  3 files changed, 36 insertions(+), 33 deletions(-)
>
> Index: src/gdb/breakpoint.c
> ===================================================================
> --- src.orig/gdb/breakpoint.c   2011-02-18 10:13:06.000000000 +0000
> +++ src/gdb/breakpoint.c        2011-02-18 10:36:37.487376996 +0000
> @@ -573,8 +573,7 @@ get_number_trailer (char **pp, int trail
>   char *p = *pp;
>
>   if (p == NULL)
> -    /* Empty line means refer to the last breakpoint.  */
> -    return breakpoint_count;
> +    return 0;
>   else if (*p == '$')
>     {
>       /* Make a copy of the name, so we can null-terminate it
> @@ -651,7 +650,7 @@ get_number (char **pp)
>    is completed.  The call that completes the range will advance
>    pointer PP past <number2>.  */
>
> -int
> +int
>  get_number_or_range (char **pp)
>  {
>   static int last_retval, end_value;
> Index: src/gdb/breakpoint.h
> ===================================================================
> --- src.orig/gdb/breakpoint.h   2011-02-08 09:31:08.000000000 +0000
> +++ src/gdb/breakpoint.h        2011-02-18 10:55:56.367376999 +0000
> @@ -1191,4 +1191,6 @@ extern struct breakpoint *iterate_over_b
>
>  extern int user_breakpoint_p (struct breakpoint *);
>
> +extern int get_number_or_range (char **pp);
> +
>  #endif /* !defined (BREAKPOINT_H) */
> Index: src/gdb/printcmd.c
> ===================================================================
> --- src.orig/gdb/printcmd.c     2011-02-01 15:27:37.000000000 +0000
> +++ src/gdb/printcmd.c  2011-02-18 11:12:56.587376996 +0000
> @@ -167,6 +167,11 @@ static struct display *display_chain;
>
>  static int display_number;
>
> +/* Walk the following statement or block through all displays.  */
> +
> +#define ALL_DISPLAYS(B)                                \
> +  for (B = display_chain; B; B = B->next)
> +
>  /* Prototypes for exported functions.  */
>
>  void output_command (char *, int);
> @@ -1555,35 +1560,26 @@ clear_displays (void)
>     }
>  }
>
> -/* Delete the auto-display number NUM.  */
> +/* Delete the auto-display DISPLAY.  */
>
>  static void
> -delete_display (int num)
> +delete_display (struct display *display)
>  {
> -  struct display *d1, *d;
> +  struct display *d;
>
> -  if (!display_chain)
> -    error (_("No display number %d."), num);
> +  gdb_assert (display != NULL);
>
> -  if (display_chain->number == num)
> -    {
> -      d1 = display_chain;
> -      display_chain = d1->next;
> -      free_display (d1);
> -    }
> -  else
> -    for (d = display_chain;; d = d->next)
> +  if (display_chain == display)
> +    display_chain = display->next;
> +
> +  ALL_DISPLAYS (d)
> +    if (d->next == display)
>       {
> -       if (d->next == 0)
> -         error (_("No display number %d."), num);
> -       if (d->next->number == num)
> -         {
> -           d1 = d->next;
> -           d->next = d1->next;
> -           free_display (d1);
> -           break;
> -         }
> +       d->next = display->next;
> +       break;
>       }
> +
> +  free_display (display);
>  }
>
>  /* Delete some values from the auto-display chain.
> @@ -1607,18 +1603,24 @@ undisplay_command (char *args, int from_
>   while (*p)
>     {
>       p1 = p;
> -      while (*p1 >= '0' && *p1 <= '9')
> -       p1++;
> -      if (*p1 && *p1 != ' ' && *p1 != '\t')
> -       error (_("Arguments must be display numbers."));
>
> -      num = atoi (p);
> +      num = get_number_or_range (&p1);
> +      if (num == 0)
> +       warning (_("bad display number at or near '%s'"), p);
> +      else
> +       {
> +         struct display *d;
>
> -      delete_display (num);
> +         ALL_DISPLAYS (d)
> +           if (d->number == num)
> +             break;
> +         if (d == NULL)
> +           printf_unfiltered (_("No display number %d.\n"), num);
> +         else
> +           delete_display (d);
> +       }
>
>       p = p1;
> -      while (*p == ' ' || *p == '\t')
> -       p++;
>     }
>   dont_repeat ();
>  }
>



-- 
"A fellow of infinite jest, of most excellent fancy."