Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch] delete a range of display numbers
@ 2011-02-18  9:47 Guillaume Leconte
  2011-02-18 10:30 ` Guillaume Leconte
                   ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Guillaume Leconte @ 2011-02-18  9:47 UTC (permalink / raw)
  To: gdb-patches

I often have a bunch of display numbers when I use gdb.  It's pretty
annoying to be unable to remove for example the N first ones I've
previously set.

This patch allow the user to do this:

(gdb) delete display 1-7

Notice that I've change the prototype of delete_display() in order
to have a silent return in case of unknown number.  If you have set
the display numbers 1, 2, 3 and 5, you can remove 2-5 without any
error messages (number 4 is missing in the list).  As a consequence,
you can also remove out of bound numbers, as in 'delete display 5-42'
without having gdb complaining.

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 29ffbf5..6f718c4 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1420,7 +1420,7 @@ x_command (char *exp, int from_tty)
       val = coerce_ref (val);
      /* In rvalue contexts, such as this, functions are coerced into
         pointers to functions.  This makes "x/i main" work.  */
-      if (/* last_format == 'i'  && */
+      if (/* last_format == 'i'  && */
         TYPE_CODE (value_type (val)) == TYPE_CODE_FUNC
          && VALUE_LVAL (val) == lval_memory)
       next_address = value_address (val);
@@ -1558,12 +1558,17 @@ clear_displays (void)
 /* Delete the auto-display number NUM.  */

 static void
-delete_display (int num)
+delete_display (int num,
+                int ignore)
 {
  struct display *d1, *d;

  if (!display_chain)
-    error (_("No display number %d."), num);
+    {
+      if (ignore)
+        return;
+      error (_("No display number %d."), num);
+    }

  if (display_chain->number == num)
    {
@@ -1575,7 +1580,11 @@ delete_display (int num)
    for (d = display_chain;; d = d->next)
      {
       if (d->next == 0)
-         error (_("No display number %d."), num);
+          {
+            if (ignore)
+              return;
+            error (_("No display number %d."), num);
+          }
       if (d->next->number == num)
         {
           d1 = d->next;
@@ -1586,6 +1595,42 @@ delete_display (int num)
      }
 }

+static int
+set_int_from_str(char *str,
+                 int *result,
+                 char *end_delim)
+{
+  char *p;
+  int ret;
+  char *occurrence;
+
+  p = str;
+  ret = -1;
+
+  if (! str)
+    goto err;
+
+  while (p && *p && (' ' == *p || '\t' == *p))
+    p++;
+
+  while (p && *p && *p >= '0' && *p <= '9')
+    p++;
+
+  if (p && *p)
+    {
+      occurrence = strpbrk(p, end_delim);
+      if (! occurrence || occurrence != p)
+        goto err;
+    }
+
+  if (result)
+    *result = atoi(str);
+
+  ret = 0;
+ err:
+  return ret;
+}
+
 /* Delete some values from the auto-display chain.
   Specify the element numbers.  */

@@ -1595,6 +1640,8 @@ undisplay_command (char *args, int from_tty)
  char *p = args;
  char *p1;
  int num;
+  int lower, upper, i;
+  char *dash;

  if (args == 0)
    {
@@ -1604,26 +1651,47 @@ undisplay_command (char *args, int from_tty)
      return;
    }

-  while (*p)
+  dash = strchr(p, '-');
+  if (dash) /* remove all the display IDs within a range */
+    {
+      if (-1 == set_int_from_str(p, &lower, "- \t"))
+        error (_("Arguments must be display numbers."));
+
+      p = dash+1;
+      while (p && *p && (' ' == *p || '\t' == *p))
+        p++;
+
+      if (-1 == set_int_from_str(p, &upper, " \t"))
+        error (_("Arguments must be display numbers."));
+
+      for (i = lower; i <= upper; i++)
+        delete_display(i, 1);
+
+      dont_repeat ();
+    }
+  else
    {
-      p1 = p;
-      while (*p1 >= '0' && *p1 <= '9')
-       p1++;
-      if (*p1 && *p1 != ' ' && *p1 != '\t')
-       error (_("Arguments must be display numbers."));
+      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 = atoi (p);

-      delete_display (num);
+          delete_display (num);

-      p = p1;
-      while (*p == ' ' || *p == '\t')
-       p++;
+          p = p1;
+          while (*p == ' ' || *p == '\t')
+            p++;
+        }
+      dont_repeat ();
    }
-  dont_repeat ();
 }

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18  9:47 [patch] delete a range of display numbers Guillaume Leconte
@ 2011-02-18 10:30 ` Guillaume Leconte
  2011-02-18 11:08 ` Eli Zaretskii
  2011-02-18 17:44 ` Michael Snyder
  2 siblings, 0 replies; 21+ messages in thread
From: Guillaume Leconte @ 2011-02-18 10:30 UTC (permalink / raw)
  To: gdb-patches

This version fits more with the indentation style, and fixes a typo (I
omitted to change a call
to delete_display()). Sorry for the noise.

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 29ffbf5..deb1b80 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1558,12 +1558,16 @@ clear_displays (void)
 /* Delete the auto-display number NUM.  */

 static void
-delete_display (int num)
+delete_display (int num, int ignore)
 {
   struct display *d1, *d;

   if (!display_chain)
-    error (_("No display number %d."), num);
+    {
+      if (ignore)
+        return;
+      error (_("No display number %d."), num);
+    }

   if (display_chain->number == num)
     {
@@ -1575,7 +1579,11 @@ delete_display (int num)
     for (d = display_chain;; d = d->next)
       {
 	if (d->next == 0)
-	  error (_("No display number %d."), num);
+          {
+            if (ignore)
+              return;
+            error (_("No display number %d."), num);
+          }
 	if (d->next->number == num)
 	  {
 	    d1 = d->next;
@@ -1586,6 +1594,40 @@ delete_display (int num)
       }
 }

+static int
+set_int_from_str (char *str, int *result, char *end_delim)
+{
+  char *p;
+  int ret;
+  char *occurrence;
+
+  p = str;
+  ret = -1;
+
+  if (! str)
+    goto err;
+
+  while (p && *p && (' ' == *p || '\t' == *p))
+    p++;
+
+  while (p && *p && *p >= '0' && *p <= '9')
+    p++;
+
+  if (p && *p)
+    {
+      occurrence = strpbrk (p, end_delim);
+      if (! occurrence || occurrence != p)
+        goto err;
+    }
+
+  if (result)
+    *result = atoi (str);
+
+  ret = 0;
+ err:
+  return ret;
+}
+
 /* Delete some values from the auto-display chain.
    Specify the element numbers.  */

@@ -1595,7 +1637,8 @@ undisplay_command (char *args, int from_tty)
   char *p = args;
   char *p1;
   int num;
-
+  int lower, upper, i;
+  char *dash;
   if (args == 0)
     {
       if (query (_("Delete all auto-display expressions? ")))
@@ -1604,23 +1647,44 @@ undisplay_command (char *args, int from_tty)
       return;
     }

-  while (*p)
+  dash = strchr(p, '-');
+  if (dash) /* remove all the display IDs within a range */
+    {
+      if (-1 == set_int_from_str(p, &lower, "- \t"))
+        error (_("Arguments must be display numbers."));
+
+      p = dash+1;
+      while (p && *p && (' ' == *p || '\t' == *p))
+        p++;
+
+      if (-1 == set_int_from_str(p, &upper, " \t"))
+        error (_("Arguments must be display numbers."));
+
+      for (i = lower; i <= upper; i++)
+        delete_display(i, 1);
+
+      dont_repeat ();
+    }
+  else
     {
-      p1 = p;
-      while (*p1 >= '0' && *p1 <= '9')
-	p1++;
-      if (*p1 && *p1 != ' ' && *p1 != '\t')
-	error (_("Arguments must be display numbers."));
+      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 = atoi (p);

-      delete_display (num);
+          delete_display (num, 0);

-      p = p1;
-      while (*p == ' ' || *p == '\t')
-	p++;
+          p = p1;
+          while (*p == ' ' || *p == '\t')
+            p++;
+        }
+      dont_repeat ();
     }
-  dont_repeat ();
 }

 /* Display a single auto-display.


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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18  9:47 [patch] delete a range of display numbers Guillaume Leconte
  2011-02-18 10:30 ` Guillaume Leconte
@ 2011-02-18 11:08 ` Eli Zaretskii
  2011-02-18 11:48   ` Guillaume Leconte
  2011-02-18 17:44 ` Michael Snyder
  2 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2011-02-18 11:08 UTC (permalink / raw)
  To: Guillaume Leconte; +Cc: gdb-patches

> Date: Fri, 18 Feb 2011 10:36:18 +0100
> From: Guillaume Leconte <guillaume.leconte@gmail.com>
> 
> Notice that I've change the prototype of delete_display() in order
> to have a silent return in case of unknown number.  If you have set
> the display numbers 1, 2, 3 and 5, you can remove 2-5 without any
> error messages (number 4 is missing in the list).  As a consequence,
> you can also remove out of bound numbers, as in 'delete display 5-42'
> without having gdb complaining.

I think it's a convenient feature, but doing this silently may not be
a good idea.  How about displaying a note about this?  Something like

  (Ignored some display numbers.)

WDYT?


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 11:08 ` Eli Zaretskii
@ 2011-02-18 11:48   ` Guillaume Leconte
  2011-02-18 12:17     ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Guillaume Leconte @ 2011-02-18 11:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

Thank you for this suggestion.  I added the modification you asked for
in the attached file.

Guillaume.

On Fri, Feb 18, 2011 at 11:30 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 18 Feb 2011 10:36:18 +0100
>> From: Guillaume Leconte <guillaume.leconte@gmail.com>
>>
>> Notice that I've change the prototype of delete_display() in order
>> to have a silent return in case of unknown number.  If you have set
>> the display numbers 1, 2, 3 and 5, you can remove 2-5 without any
>> error messages (number 4 is missing in the list).  As a consequence,
>> you can also remove out of bound numbers, as in 'delete display 5-42'
>> without having gdb complaining.
>
> I think it's a convenient feature, but doing this silently may not be
> a good idea.  How about displaying a note about this?  Something like
>
>  (Ignored some display numbers.)
>
> WDYT?
>



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

[-- Attachment #2: delete-display-range.patch --]
[-- Type: text/x-patch, Size: 3478 bytes --]

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 29ffbf5..8ab1a54 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1557,13 +1557,18 @@ clear_displays (void)
 
 /* Delete the auto-display number NUM.  */
 
-static void
-delete_display (int num)
+static int
+delete_display (int num, int ignore)
 {
   struct display *d1, *d;
 
   if (!display_chain)
-    error (_("No display number %d."), num);
+    {
+      if (ignore)
+        return 1;
+      printf_filtered (_("No display number %d.\n"), num);
+      return -1;
+    }
 
   if (display_chain->number == num)
     {
@@ -1575,7 +1580,12 @@ delete_display (int num)
     for (d = display_chain;; d = d->next)
       {
 	if (d->next == 0)
-	  error (_("No display number %d."), num);
+          {
+            if (ignore)
+                return 1;
+            printf_filtered (_("No display number %d.\n"), num);
+            return -1;
+          }
 	if (d->next->number == num)
 	  {
 	    d1 = d->next;
@@ -1584,6 +1594,42 @@ delete_display (int num)
 	    break;
 	  }
       }
+
+  return 0;
+}
+
+static int
+set_int_from_str(char *str, int *result, char *end_delim)
+{
+  char *p;
+  int ret;
+  char *occurrence;
+
+  p = str;
+  ret = -1;
+
+  if (! str)
+    goto err;
+
+  while (p && *p && (' ' == *p || '\t' == *p))
+    p++;
+
+  while (p && *p && *p >= '0' && *p <= '9')
+    p++;
+
+  if (p && *p)
+    {
+      occurrence = strpbrk(p, end_delim);
+      if (! occurrence || occurrence != p)
+        goto err;
+    }
+
+  if (result)
+    *result = atoi(str);
+
+  ret = 0;
+ err:
+  return ret;
 }
 
 /* Delete some values from the auto-display chain.
@@ -1595,6 +1641,9 @@ undisplay_command (char *args, int from_tty)
   char *p = args;
   char *p1;
   int num;
+  int lower, upper, i;
+  char *dash;
+  int err;
 
   if (args == 0)
     {
@@ -1604,23 +1653,52 @@ undisplay_command (char *args, int from_tty)
       return;
     }
 
-  while (*p)
+  dash = strchr(p, '-');
+  if (dash) /* remove all the display IDs within a range */
+    {
+      err = 0;
+
+      if (-1 == set_int_from_str(p, &lower, "- \t"))
+        error (_("Arguments must be display numbers."));
+
+      p = dash+1;
+      while (p && *p && (' ' == *p || '\t' == *p))
+        p++;
+
+      if (-1 == set_int_from_str(p, &upper, " \t"))
+        error (_("Arguments must be display numbers."));
+
+      for (i = lower; i <= upper; i++)
+        err += delete_display (i, 1);
+
+      /* since we called delete_display with 1 as the second argument,
+         we know that err is 0 or > 0.  It can not be negative. */
+      if (err)
+        printf_filtered (_("(Ignored some display numbers)\n"));
+
+      dont_repeat ();
+    }
+  else
     {
-      p1 = p;
-      while (*p1 >= '0' && *p1 <= '9')
-	p1++;
-      if (*p1 && *p1 != ' ' && *p1 != '\t')
-	error (_("Arguments must be display numbers."));
+      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 = atoi (p);
 
-      delete_display (num);
+          if (-1 == delete_display (num, 0))
+            return;
 
-      p = p1;
-      while (*p == ' ' || *p == '\t')
-	p++;
+          p = p1;
+          while (*p == ' ' || *p == '\t')
+            p++;
+        }
+      dont_repeat ();
     }
-  dont_repeat ();
 }
 
 /* Display a single auto-display.  

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 11:48   ` Guillaume Leconte
@ 2011-02-18 12:17     ` Pedro Alves
  2011-02-18 14:41       ` Guillaume Leconte
                         ` (2 more replies)
  0 siblings, 3 replies; 21+ messages in thread
From: Pedro Alves @ 2011-02-18 12:17 UTC (permalink / raw)
  To: gdb-patches; +Cc: Guillaume Leconte, Eli Zaretskii

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 ();
 }


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 12:17     ` Pedro Alves
@ 2011-02-18 14:41       ` Guillaume Leconte
  2011-02-18 15:14       ` Tom Tromey
  2011-02-18 17:52       ` Michael Snyder
  2 siblings, 0 replies; 21+ messages in thread
From: Guillaume Leconte @ 2011-02-18 14:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Eli Zaretskii

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."


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 12:17     ` Pedro Alves
  2011-02-18 14:41       ` Guillaume Leconte
@ 2011-02-18 15:14       ` Tom Tromey
  2011-02-18 15:58         ` Pedro Alves
  2011-02-18 17:52       ` Michael Snyder
  2 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2011-02-18 15:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Guillaume Leconte, Eli Zaretskii

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> The "delete" breakpoint command accepts ranges as
Pedro> well, and even has code that handles convenience
Pedro> variables mixed with the numbers.  I think we should
Pedro> reuse that instead of re-adding code that parses ranges.
Pedro> Might as well make the "delete display" command
Pedro> implementation look more like the "delete" command
Pedro> implementation.  Here's a quick cut at it.

I've been thinking of starting a cli/cli-utils.c file and putting shared
CLI parsing code there.  This seems like a good candidate, WDYT?
I can do the moving.

Pedro>    if (p == NULL)
Pedro> -    /* Empty line means refer to the last breakpoint.  */
Pedro> -    return breakpoint_count;
Pedro> +    return 0;

I was surprised that this didn't imply any other changes, but I looked
at a bunch of calls into this code and I couldn't see anything.

Tom


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 15:14       ` Tom Tromey
@ 2011-02-18 15:58         ` Pedro Alves
  2011-02-18 16:48           ` Tom Tromey
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2011-02-18 15:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey, Guillaume Leconte, Eli Zaretskii

On Friday 18 February 2011 15:14:04, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> Pedro> The "delete" breakpoint command accepts ranges as
> Pedro> well, and even has code that handles convenience
> Pedro> variables mixed with the numbers.  I think we should
> Pedro> reuse that instead of re-adding code that parses ranges.
> Pedro> Might as well make the "delete display" command
> Pedro> implementation look more like the "delete" command
> Pedro> implementation.  Here's a quick cut at it.
> 
> I've been thinking of starting a cli/cli-utils.c file and putting shared
> CLI parsing code there.  This seems like a good candidate, WDYT?

Yeah.  Sounds good.

> I can do the moving.

Thanks.

> 
> Pedro>    if (p == NULL)
> Pedro> -    /* Empty line means refer to the last breakpoint.  */
> Pedro> -    return breakpoint_count;
> Pedro> +    return 0;
> 
> I was surprised that this didn't imply any other changes, but I looked
> at a bunch of calls into this code and I couldn't see anything.

Yeah, sorry I should have said I also looked and couldn't find
where's that being used.  It look like a dead path at this point.
I'll do a testrun with a gdb_assert in place to be a bit more sure.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 15:58         ` Pedro Alves
@ 2011-02-18 16:48           ` Tom Tromey
  2011-02-18 16:57             ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2011-02-18 16:48 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Guillaume Leconte, Eli Zaretskii

Pedro> Yeah, sorry I should have said I also looked and couldn't find
Pedro> where's that being used.  It look like a dead path at this point.
Pedro> I'll do a testrun with a gdb_assert in place to be a bit more sure.

I think the last user was probably "commands" without an argument, and
then my commands-for-rbreak change modified this code path.

Tom


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 16:48           ` Tom Tromey
@ 2011-02-18 16:57             ` Pedro Alves
  2011-02-18 17:41               ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2011-02-18 16:57 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, Guillaume Leconte, Eli Zaretskii

On Friday 18 February 2011 16:15:26, Tom Tromey wrote:
> Pedro> Yeah, sorry I should have said I also looked and couldn't find
> Pedro> where's that being used.  It look like a dead path at this point.
> Pedro> I'll do a testrun with a gdb_assert in place to be a bit more sure.
> 
> I think the last user was probably "commands" without an argument, and
> then my commands-for-rbreak change modified this code path.

Okay, thanks, the gdb_assert never triggered, so I just
removed the support for NULL pp completely.

Here's what I tested&applied.

Thanks for the initial patch Guillaume.

-- 
Pedro Alves

2011-02-18  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* breakpoint.c (get_number_trailer): No longer accept a NULL PP.
	* breakpoint.h (get_number_or_range): Declare.
	* printcmd.c (ALL_DISPLAYS): Declare.
	(delete_display): Reimplement taking a display pointer.
	(undisplay_command): Accept a range of displays to delete, using
	get_number_or_range.

---
 gdb/breakpoint.c |    9 +------
 gdb/breakpoint.h |    2 +
 gdb/printcmd.c   |   62 ++++++++++++++++++++++++++++---------------------------
 3 files changed, 36 insertions(+), 37 deletions(-)

Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c	2011-02-18 16:27:51.000000000 +0000
+++ src/gdb/breakpoint.c	2011-02-18 16:30:02.627376997 +0000
@@ -561,8 +561,6 @@ struct program_space *default_breakpoint
    name of a convenience variable.  Making it an expression wouldn't
    work well for map_breakpoint_numbers (e.g. "4 + 5 + 6").
 
-   If the string is a NULL pointer, that denotes the last breakpoint.
-   
    TRAILER is a character which can be found after the number; most
    commonly this is `-'.  If you don't want a trailer, use \0.  */
 
@@ -572,10 +570,7 @@ get_number_trailer (char **pp, int trail
   int retval = 0;	/* default */
   char *p = *pp;
 
-  if (p == NULL)
-    /* Empty line means refer to the last breakpoint.  */
-    return breakpoint_count;
-  else if (*p == '$')
+  if (*p == '$')
     {
       /* Make a copy of the name, so we can null-terminate it
          to pass to lookup_internalvar().  */
@@ -651,7 +646,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-18 16:29:47.000000000 +0000
+++ src/gdb/breakpoint.h	2011-02-18 16:30:02.627376997 +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-18 16:29:47.000000000 +0000
+++ src/gdb/printcmd.c	2011-02-18 16:30:02.677376997 +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 ();
 }


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 16:57             ` Pedro Alves
@ 2011-02-18 17:41               ` Eli Zaretskii
  2011-02-18 17:54                 ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2011-02-18 17:41 UTC (permalink / raw)
  To: Pedro Alves; +Cc: tromey, gdb-patches, guillaume.leconte

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Fri, 18 Feb 2011 16:49:46 +0000
> Cc: gdb-patches@sourceware.org,
>  Guillaume Leconte <guillaume.leconte@gmail.com>,
>  Eli Zaretskii <eliz@gnu.org>
> 
> Here's what I tested&applied.

Thanks.  I think we should document the range of numbers feature.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18  9:47 [patch] delete a range of display numbers Guillaume Leconte
  2011-02-18 10:30 ` Guillaume Leconte
  2011-02-18 11:08 ` Eli Zaretskii
@ 2011-02-18 17:44 ` Michael Snyder
  2 siblings, 0 replies; 21+ messages in thread
From: Michael Snyder @ 2011-02-18 17:44 UTC (permalink / raw)
  To: Guillaume Leconte; +Cc: gdb-patches

Guillaume Leconte wrote:
> I often have a bunch of display numbers when I use gdb.  It's pretty
> annoying to be unable to remove for example the N first ones I've
> previously set.
> 
> This patch allow the user to do this:
> 
> (gdb) delete display 1-7
> 
> Notice that I've change the prototype of delete_display() in order
> to have a silent return in case of unknown number.  If you have set
> the display numbers 1, 2, 3 and 5, you can remove 2-5 without any
> error messages (number 4 is missing in the list).  As a consequence,
> you can also remove out of bound numbers, as in 'delete display 5-42'
> without having gdb complaining.

I'm concerned about this.  What happens if I want to delete 24, but
by mistake I type "delete display 42"?  If gdb fails silently, I will
not be informed of my mistake.


> 
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index 29ffbf5..6f718c4 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -1420,7 +1420,7 @@ x_command (char *exp, int from_tty)
>        val = coerce_ref (val);
>       /* In rvalue contexts, such as this, functions are coerced into
>          pointers to functions.  This makes "x/i main" work.  */
> -      if (/* last_format == 'i'  && */
> +      if (/* last_format == 'i'  && */
>          TYPE_CODE (value_type (val)) == TYPE_CODE_FUNC
>           && VALUE_LVAL (val) == lval_memory)
>        next_address = value_address (val);
> @@ -1558,12 +1558,17 @@ clear_displays (void)
>  /* Delete the auto-display number NUM.  */
> 
>  static void
> -delete_display (int num)
> +delete_display (int num,
> +                int ignore)
>  {
>   struct display *d1, *d;
> 
>   if (!display_chain)
> -    error (_("No display number %d."), num);
> +    {
> +      if (ignore)
> +        return;
> +      error (_("No display number %d."), num);
> +    }
> 
>   if (display_chain->number == num)
>     {
> @@ -1575,7 +1580,11 @@ delete_display (int num)
>     for (d = display_chain;; d = d->next)
>       {
>        if (d->next == 0)
> -         error (_("No display number %d."), num);
> +          {
> +            if (ignore)
> +              return;
> +            error (_("No display number %d."), num);
> +          }
>        if (d->next->number == num)
>          {
>            d1 = d->next;
> @@ -1586,6 +1595,42 @@ delete_display (int num)
>       }
>  }
> 
> +static int
> +set_int_from_str(char *str,
> +                 int *result,
> +                 char *end_delim)
> +{
> +  char *p;
> +  int ret;
> +  char *occurrence;
> +
> +  p = str;
> +  ret = -1;
> +
> +  if (! str)
> +    goto err;
> +
> +  while (p && *p && (' ' == *p || '\t' == *p))
> +    p++;
> +
> +  while (p && *p && *p >= '0' && *p <= '9')
> +    p++;
> +
> +  if (p && *p)
> +    {
> +      occurrence = strpbrk(p, end_delim);
> +      if (! occurrence || occurrence != p)
> +        goto err;
> +    }
> +
> +  if (result)
> +    *result = atoi(str);
> +
> +  ret = 0;
> + err:
> +  return ret;
> +}
> +
>  /* Delete some values from the auto-display chain.
>    Specify the element numbers.  */
> 
> @@ -1595,6 +1640,8 @@ undisplay_command (char *args, int from_tty)
>   char *p = args;
>   char *p1;
>   int num;
> +  int lower, upper, i;
> +  char *dash;
> 
>   if (args == 0)
>     {
> @@ -1604,26 +1651,47 @@ undisplay_command (char *args, int from_tty)
>       return;
>     }
> 
> -  while (*p)
> +  dash = strchr(p, '-');
> +  if (dash) /* remove all the display IDs within a range */
> +    {
> +      if (-1 == set_int_from_str(p, &lower, "- \t"))
> +        error (_("Arguments must be display numbers."));
> +
> +      p = dash+1;
> +      while (p && *p && (' ' == *p || '\t' == *p))
> +        p++;
> +
> +      if (-1 == set_int_from_str(p, &upper, " \t"))
> +        error (_("Arguments must be display numbers."));
> +
> +      for (i = lower; i <= upper; i++)
> +        delete_display(i, 1);
> +
> +      dont_repeat ();
> +    }
> +  else
>     {
> -      p1 = p;
> -      while (*p1 >= '0' && *p1 <= '9')
> -       p1++;
> -      if (*p1 && *p1 != ' ' && *p1 != '\t')
> -       error (_("Arguments must be display numbers."));
> +      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 = atoi (p);
> 
> -      delete_display (num);
> +          delete_display (num);
> 
> -      p = p1;
> -      while (*p == ' ' || *p == '\t')
> -       p++;
> +          p = p1;
> +          while (*p == ' ' || *p == '\t')
> +            p++;
> +        }
> +      dont_repeat ();
>     }
> -  dont_repeat ();
>  }
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 12:17     ` Pedro Alves
  2011-02-18 14:41       ` Guillaume Leconte
  2011-02-18 15:14       ` Tom Tromey
@ 2011-02-18 17:52       ` Michael Snyder
  2011-02-18 17:57         ` Pedro Alves
  2 siblings, 1 reply; 21+ messages in thread
From: Michael Snyder @ 2011-02-18 17:52 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Guillaume Leconte, Eli Zaretskii

Pedro Alves 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.
> 

Yes, I was wishing that that code were reuseable, so that I could
use it for info threads.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 17:41               ` Eli Zaretskii
@ 2011-02-18 17:54                 ` Pedro Alves
  2011-02-18 17:54                   ` Michael Snyder
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2011-02-18 17:54 UTC (permalink / raw)
  To: gdb-patches, Eli Zaretskii; +Cc: tromey, guillaume.leconte

On Friday 18 February 2011 17:39:28, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Fri, 18 Feb 2011 16:49:46 +0000
> > Cc: gdb-patches@sourceware.org,
> >  Guillaume Leconte <guillaume.leconte@gmail.com>,
> >  Eli Zaretskii <eliz@gnu.org>
> > 
> > Here's what I tested&applied.
> 
> Thanks.  I think we should document the range of numbers feature.

Yeah.  I'm looking through the manual to see where
is the fact that "delete breakpoints" accepts ranges
documented, and not finding it.  I see that "thread apply"
documents it (candidate for get_number_or_range).

"disable display" and "enable display" should get the
same treatment to accept ranges.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 17:54                 ` Pedro Alves
@ 2011-02-18 17:54                   ` Michael Snyder
  2011-02-18 18:06                     ` Pedro Alves
  2011-02-18 18:17                     ` [patch] delete a range of display numbers Eli Zaretskii
  0 siblings, 2 replies; 21+ messages in thread
From: Michael Snyder @ 2011-02-18 17:54 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Eli Zaretskii, tromey, guillaume.leconte

Pedro Alves wrote:
> On Friday 18 February 2011 17:39:28, Eli Zaretskii wrote:
>>> From: Pedro Alves <pedro@codesourcery.com>
>>> Date: Fri, 18 Feb 2011 16:49:46 +0000
>>> Cc: gdb-patches@sourceware.org,
>>>  Guillaume Leconte <guillaume.leconte@gmail.com>,
>>>  Eli Zaretskii <eliz@gnu.org>
>>>
>>> Here's what I tested&applied.
>> Thanks.  I think we should document the range of numbers feature.
> 
> Yeah.  I'm looking through the manual to see where
> is the fact that "delete breakpoints" accepts ranges
> documented, and not finding it.  I see that "thread apply"
> documents it (candidate for get_number_or_range).
> 
> "disable display" and "enable display" should get the
> same treatment to accept ranges.
> 


delete [breakpoints] [range...]
     Delete the breakpoints, watchpoints, or catchpoints of the 
breakpoint ranges specified as arguments. If no argument is specified, 
delete all breakpoints (gdb asks confirmation, unless you have set 
confirm off). You can abbreviate this command as d.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 17:52       ` Michael Snyder
@ 2011-02-18 17:57         ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2011-02-18 17:57 UTC (permalink / raw)
  To: gdb-patches; +Cc: Michael Snyder, Guillaume Leconte, Eli Zaretskii

On Friday 18 February 2011 17:44:26, Michael Snyder wrote:
> Yes, I was wishing that that code were reuseable, so that I could
> use it for info threads.

Well, when it ain't, we just got to make it so.  :-)
It's reusable now.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 17:54                   ` Michael Snyder
@ 2011-02-18 18:06                     ` Pedro Alves
  2011-03-14 21:24                       ` [patch+docs] make 'disable|enable display' also accept ranges (Re: [patch] delete a range of display numbers) Pedro Alves
  2011-02-18 18:17                     ` [patch] delete a range of display numbers Eli Zaretskii
  1 sibling, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2011-02-18 18:06 UTC (permalink / raw)
  To: gdb-patches; +Cc: Michael Snyder, Eli Zaretskii, tromey, guillaume.leconte

On Friday 18 February 2011 17:54:01, Michael Snyder wrote:
> delete [breakpoints] [range...]
>      Delete the breakpoints, watchpoints, or catchpoints of the 
> breakpoint ranges specified as arguments. If no argument is specified, 
> delete all breakpoints (gdb asks confirmation, unless you have set 
> confirm off). You can abbreviate this command as d.

I need new glasses.  Thanks!

The help text could use an update though:

(gdb) help delete breakpoints 
Delete some breakpoints or auto-display expressions.
Arguments are breakpoint numbers with spaces in between.
To delete all breakpoints, give no argument.
This command may be abbreviated "delete".

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch] delete a range of display numbers
  2011-02-18 17:54                   ` Michael Snyder
  2011-02-18 18:06                     ` Pedro Alves
@ 2011-02-18 18:17                     ` Eli Zaretskii
  1 sibling, 0 replies; 21+ messages in thread
From: Eli Zaretskii @ 2011-02-18 18:17 UTC (permalink / raw)
  To: Michael Snyder; +Cc: pedro, gdb-patches, tromey, guillaume.leconte

> Date: Fri, 18 Feb 2011 09:54:01 -0800
> From: Michael Snyder <msnyder@vmware.com>
> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, 
>  Eli Zaretskii <eliz@gnu.org>,
>  "tromey@redhat.com" <tromey@redhat.com>, 
>  "guillaume.leconte@gmail.com" <guillaume.leconte@gmail.com>
> 
> delete [breakpoints] [range...]

But RANGE is not explained anywhere.  It's probably a good place to
explain it in this node, and then other nodes could point there.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [patch+docs] make 'disable|enable display' also accept ranges (Re: [patch] delete a range of display numbers)
  2011-02-18 18:06                     ` Pedro Alves
@ 2011-03-14 21:24                       ` Pedro Alves
  2011-03-14 21:34                         ` Eli Zaretskii
  0 siblings, 1 reply; 21+ messages in thread
From: Pedro Alves @ 2011-03-14 21:24 UTC (permalink / raw)
  To: gdb-patches; +Cc: Michael Snyder, Eli Zaretskii, tromey, guillaume.leconte

I finally took a bit to come back to this:

On Friday 18 February 2011 17:52:02, Pedro Alves wrote:
> "disable display" and "enable display" should get the
> same treatment to accept ranges.

... and that's what the code patch does.

The docs patch tweaks the commands' ("undisplay" too)
docs to mention what they now accept.  Does it look okay?

Pedro Alves

2011-03-14  Pedro Alves  <pedro@codesourcery.com>

	gdb/
	* printcmd.c (ALL_DISPLAYS_SAFE): New.
	(map_display_numbers): New.
	(do_delete_display): New.
	(undisplay_command): Use map_display_numbers.
	(do_enable_disable_display): New.
	(enable_disable_display_command): New function.
	(enable_display): Delete.
	(enable_display_command): New.
	(disable_display_command): Reimplement.
	(_initialize_printcmd): Adjust "enable display" command to use
	`enable_display_command' as callback.

	gdb/doc/
	* gdb.texinfo (Auto Display) <undisplay, enable display, disable
	display>: Explain that the commands accept number ranges.

---
 gdb/doc/gdb.texinfo |   16 ++++-
 gdb/printcmd.c      |  150 +++++++++++++++++++++++++++-------------------------
 2 files changed, 94 insertions(+), 72 deletions(-)

Index: src/gdb/printcmd.c
===================================================================
--- src.orig/gdb/printcmd.c	2011-03-14 12:40:01.000000000 +0000
+++ src/gdb/printcmd.c	2011-03-14 21:12:20.148353561 +0000
@@ -168,11 +168,18 @@ static struct display *display_chain;
 
 static int display_number;
 
-/* Walk the following statement or block through all displays.  */
+/* Walk the following statement or block through all displays.
+   ALL_DISPLAYS_SAFE does so even if the statement deletes the current
+   display.  */
 
 #define ALL_DISPLAYS(B)				\
   for (B = display_chain; B; B = B->next)
 
+#define ALL_DISPLAYS_SAFE(B,TMP)		\
+  for (B = display_chain;			\
+       B ? (TMP = B->next, 1): 0;		\
+       B = TMP)
+
 /* Prototypes for exported functions.  */
 
 void output_command (char *, int);
@@ -1583,24 +1590,24 @@ delete_display (struct display *display)
   free_display (display);
 }
 
-/* Delete some values from the auto-display chain.
-   Specify the element numbers.  */
+/* Call FUNCTION on each of the displays whose numbers are given in
+   ARGS.  DATA is passed unmodified to FUNCTION.  */
 
 static void
-undisplay_command (char *args, int from_tty)
+map_display_numbers (char *args,
+		     void (*function) (struct display *,
+				       void *),
+		     void *data)
 {
-  int num;
   struct get_number_or_range_state state;
+  struct display *b, *tmp;
+  int num;
 
-  if (args == 0)
-    {
-      if (query (_("Delete all auto-display expressions? ")))
-	clear_displays ();
-      dont_repeat ();
-      return;
-    }
+  if (args == NULL)
+    error_no_arg (_("one or more display numbers"));
 
   init_number_or_range (&state, args);
+
   while (!state.finished)
     {
       char *p = state.string;
@@ -1610,17 +1617,44 @@ undisplay_command (char *args, int from_
 	warning (_("bad display number at or near '%s'"), p);
       else
 	{
-	  struct display *d;
+	  struct display *d, *tmp;
 
-	  ALL_DISPLAYS (d)
+	  ALL_DISPLAYS_SAFE (d, tmp)
 	    if (d->number == num)
 	      break;
 	  if (d == NULL)
 	    printf_unfiltered (_("No display number %d.\n"), num);
 	  else
-	    delete_display (d);
+	    function (d, data);
 	}
     }
+}
+
+/* Callback for map_display_numbers, that deletes a display.  */
+
+static void
+do_delete_display (struct display *d, void *data)
+{
+  delete_display (d);
+}
+
+/* "undisplay" command.  */
+
+static void
+undisplay_command (char *args, int from_tty)
+{
+  int num;
+  struct get_number_or_range_state state;
+
+  if (args == NULL)
+    {
+      if (query (_("Delete all auto-display expressions? ")))
+	clear_displays ();
+      dont_repeat ();
+      return;
+    }
+
+  map_display_numbers (args, do_delete_display, NULL);
   dont_repeat ();
 }
 
@@ -1823,71 +1857,47 @@ Num Enb Expression\n"));
     }
 }
 
+/* Callback fo map_display_numbers, that enables or disable the passed
+   in display D.  */
+
 static void
-enable_display (char *args, int from_tty)
+do_enable_disable_display (struct display *d, void *data)
 {
-  char *p = args;
-  char *p1;
-  int num;
-  struct display *d;
+  d->enabled_p = *(int *) data;
+}
+
+/* Implamentation of both the "disable display" and "enable display"
+   commands.  ENABLE decides what to do.  */
 
-  if (p == 0)
+static void
+enable_disable_display_command (char *args, int from_tty, int enable)
+{
+  if (args == NULL)
     {
-      for (d = display_chain; d; d = d->next)
-	d->enabled_p = 1;
+      struct display *d;
+
+      ALL_DISPLAYS (d)
+	d->enabled_p = enable;
+      return;
     }
-  else
-    while (*p)
-      {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be display numbers."));
-
-	num = atoi (p);
-
-	for (d = display_chain; d; d = d->next)
-	  if (d->number == num)
-	    {
-	      d->enabled_p = 1;
-	      goto win;
-	    }
-	printf_unfiltered (_("No display number %d.\n"), num);
-      win:
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
-      }
+
+  map_display_numbers (args, do_enable_disable_display, &enable);
 }
 
+/* The "enable display" command.  */
+
 static void
-disable_display_command (char *args, int from_tty)
+enable_display_command (char *args, int from_tty)
 {
-  char *p = args;
-  char *p1;
-  struct display *d;
-
-  if (p == 0)
-    {
-      for (d = display_chain; d; d = d->next)
-	d->enabled_p = 0;
-    }
-  else
-    while (*p)
-      {
-	p1 = p;
-	while (*p1 >= '0' && *p1 <= '9')
-	  p1++;
-	if (*p1 && *p1 != ' ' && *p1 != '\t')
-	  error (_("Arguments must be display numbers."));
+  enable_disable_display_command (args, from_tty, 1);
+}
 
-	disable_display (atoi (p));
+/* The "disable display" command.  */
 
-	p = p1;
-	while (*p == ' ' || *p == '\t')
-	  p++;
-      }
+static void
+disable_display_command (char *args, int from_tty)
+{
+  enable_disable_display_command (args, from_tty, 0);
 }
 
 /* display_chain items point to blocks and expressions.  Some expressions in
@@ -2749,7 +2759,7 @@ and examining is done as in the \"x\" co
 With no argument, display all currently requested auto-display expressions.\n\
 Use \"undisplay\" to cancel display requests previously made."));
 
-  add_cmd ("display", class_vars, enable_display, _("\
+  add_cmd ("display", class_vars, enable_display_command, _("\
 Enable some expressions to be displayed when program stops.\n\
 Arguments are the code numbers of the expressions to resume displaying.\n\
 No argument means enable all automatic-display expressions.\n\
Index: src/gdb/doc/gdb.texinfo
===================================================================
--- src.orig/gdb/doc/gdb.texinfo	2011-03-14 20:47:04.000000000 +0000
+++ src/gdb/doc/gdb.texinfo	2011-03-14 20:47:26.598353561 +0000
@@ -7647,7 +7647,11 @@ is a common name for the program counter
 @kindex undisplay
 @item undisplay @var{dnums}@dots{}
 @itemx delete display @var{dnums}@dots{}
-Remove item numbers @var{dnums} from the list of expressions to display.
+Remove items from the list of expressions to display.  Specify the
+numbers of the displays that you want affected with the command
+argument @var{dnums}.  It can be a single display number, one of the
+numbers shown in the first field of the @samp{info display} display;
+or it could be a range of display numbers, as in @code{2-4}.
 
 @code{undisplay} does not repeat if you press @key{RET} after using it.
 (Otherwise you would just get the error @samp{No display number @dots{}}.)
@@ -7656,12 +7660,20 @@ Remove item numbers @var{dnums} from the
 @item disable display @var{dnums}@dots{}
 Disable the display of item numbers @var{dnums}.  A disabled display
 item is not printed automatically, but is not forgotten.  It may be
-enabled again later.
+enabled again later.  Specify the numbers of the displays that you
+want affected with the command argument @var{dnums}.  It can be a
+single display number, one of the numbers shown in the first field of
+the @samp{info display} display; or it could be a range of display
+numbers, as in @code{2-4}.
 
 @kindex enable display
 @item enable display @var{dnums}@dots{}
 Enable display of item numbers @var{dnums}.  It becomes effective once
 again in auto display of its expression, until you specify otherwise.
+Specify the numbers of the displays that you want affected with the
+command argument @var{dnums}.  It can be a single display number, one
+of the numbers shown in the first field of the @samp{info display}
+display; or it could be a range of display numbers, as in @code{2-4}.
 
 @item display
 Display the current values of the expressions on the list, just as is


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch+docs] make 'disable|enable display' also accept ranges (Re: [patch] delete a range of display numbers)
  2011-03-14 21:24                       ` [patch+docs] make 'disable|enable display' also accept ranges (Re: [patch] delete a range of display numbers) Pedro Alves
@ 2011-03-14 21:34                         ` Eli Zaretskii
  2011-03-15 14:43                           ` Pedro Alves
  0 siblings, 1 reply; 21+ messages in thread
From: Eli Zaretskii @ 2011-03-14 21:34 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, msnyder, tromey, guillaume.leconte

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Mon, 14 Mar 2011 21:16:13 +0000
> Cc: Michael Snyder <msnyder@vmware.com>,
>  Eli Zaretskii <eliz@gnu.org>,
>  "tromey@redhat.com" <tromey@redhat.com>,
>  "guillaume.leconte@gmail.com" <guillaume.leconte@gmail.com>
> 
> The docs patch tweaks the commands' ("undisplay" too)
> docs to mention what they now accept.  Does it look okay?

Yes, it's fine.  Thanks.


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [patch+docs] make 'disable|enable display' also accept ranges (Re: [patch] delete a range of display numbers)
  2011-03-14 21:34                         ` Eli Zaretskii
@ 2011-03-15 14:43                           ` Pedro Alves
  0 siblings, 0 replies; 21+ messages in thread
From: Pedro Alves @ 2011-03-15 14:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches, msnyder, tromey, guillaume.leconte

On Monday 14 March 2011 21:26:32, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> >
> > The docs patch tweaks the commands' ("undisplay" too)
> > docs to mention what they now accept.  Does it look okay?
> 
> Yes, it's fine.  Thanks.

Thanks, applied.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2011-03-15 14:06 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-18  9:47 [patch] delete a range of display numbers Guillaume Leconte
2011-02-18 10:30 ` Guillaume Leconte
2011-02-18 11:08 ` Eli Zaretskii
2011-02-18 11:48   ` Guillaume Leconte
2011-02-18 12:17     ` Pedro Alves
2011-02-18 14:41       ` Guillaume Leconte
2011-02-18 15:14       ` Tom Tromey
2011-02-18 15:58         ` Pedro Alves
2011-02-18 16:48           ` Tom Tromey
2011-02-18 16:57             ` Pedro Alves
2011-02-18 17:41               ` Eli Zaretskii
2011-02-18 17:54                 ` Pedro Alves
2011-02-18 17:54                   ` Michael Snyder
2011-02-18 18:06                     ` Pedro Alves
2011-03-14 21:24                       ` [patch+docs] make 'disable|enable display' also accept ranges (Re: [patch] delete a range of display numbers) Pedro Alves
2011-03-14 21:34                         ` Eli Zaretskii
2011-03-15 14:43                           ` Pedro Alves
2011-02-18 18:17                     ` [patch] delete a range of display numbers Eli Zaretskii
2011-02-18 17:52       ` Michael Snyder
2011-02-18 17:57         ` Pedro Alves
2011-02-18 17:44 ` Michael Snyder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox