[PATCH v5 1/4] Allow enabling/disabling breakpoint location ranges

[PATCH v5 1/4] Allow enabling/disabling breakpoint location ranges

[Pedro Alves]

From: Xavier Roirand <roirand@adacore.com>

When a breakpoint has multiple locations, like e.g.:

 Num  Type       Disp Enb  Address    What
 1    breakpoint keep y    <MULTIPLE>
 1.1                  y    0x080486a2 in void foo<int>()...
 1.2                  y    0x080486ca in void foo<double>()...
 [....]
 1.5                  y    0x080487fa in void foo<long>()...

it's possible to enable/disable the individual locations using the
'<breakpoint_number>.<location_number>' syntax, like e.g.:

 (gdb) disable 1.2 1.3 1.4 1.5

That's inconvenient when you have a long list of locations to disable,
however.

This patch adds shorthand for the above, by making it possible to
specify a range of locations with the following syntax (similar to
thread id ranges):

 <breakpoint_number>.<first_location_number>-<last_location_number>

For example, the command above can now be simplified to:

 (gdb) disable 1.2-5

gdb/ChangeLog:
2017-10-26  Xavier Roirand  <roirand@adacore.com>
	    Pedro Alves  <palves@redhat.com>

	* breakpoint.c (map_breakpoint_number_range): New, factored out
	from ...
	(map_breakpoint_numbers): ... here.
	(find_location_by_number): Change parameters from string to
	breakpoint number and location.
	(extract_bp_number_and_location): New function.
	(enable_disable_bp_num_loc)
	(enable_disable_breakpoint_location_range)
	(enable_disable_command): New functions, factored out ...
	(enable_command, disable_command): ... these functions, and
	adjusted to support ranges.
	* NEWS: Document enable/disable breakpoint location range feature.

gdb/doc/ChangeLog:
2017-10-26  Xavier Roirand  <roirand@adacore.com>
	    Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Set Breaks): Document support for breakpoint
	location ranges in the enable/disable commands.

gdb/testsuite/ChangeLog:
2017-10-26  Xavier Roirand  <roirand@adacore.com>
	    Pedro Alves  <palves@redhat.com>

	* gdb.base/ena-dis-br.exp: Add reference to
	gdb.cp/ena-dis-br-range.exp.
	* gdb.cp/ena-dis-br-range.exp: New file.
	* gdb.cp/ena-dis-br-range.cc: New file.
---
 gdb/doc/gdb.texinfo                       |  18 +-
 gdb/NEWS                                  |   3 +
 gdb/breakpoint.c                          | 340 ++++++++++++++++++++----------
 gdb/testsuite/gdb.base/ena-dis-br.exp     |   3 +
 gdb/testsuite/gdb.cp/ena-dis-br-range.cc  |  66 ++++++
 gdb/testsuite/gdb.cp/ena-dis-br-range.exp | 134 ++++++++++++
 6 files changed, 443 insertions(+), 121 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/ena-dis-br-range.cc
 create mode 100644 gdb/testsuite/gdb.cp/ena-dis-br-range.exp

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bfeb7a9..29d4789 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -3927,15 +3927,17 @@ Num     Type           Disp Enb  Address    What
 1.2                         y    0x080486ca in void foo<double>() at t.cc:8
 @end smallexample
 
-Each location can be individually enabled or disabled by passing
+You cannot delete the individual locations from a breakpoint.  However,
+each location can be individually enabled or disabled by passing
 @var{breakpoint-number}.@var{location-number} as argument to the
-@code{enable} and @code{disable} commands.  Note that you cannot
-delete the individual locations from the list, you can only delete the
-entire list of locations that belong to their parent breakpoint (with
-the @kbd{delete @var{num}} command, where @var{num} is the number of
-the parent breakpoint, 1 in the above example).  Disabling or enabling
-the parent breakpoint (@pxref{Disabling}) affects all of the locations
-that belong to that breakpoint.
+@code{enable} and @code{disable} commands.  It's also possible to
+@code{enable} and @code{disable} a range of @var{location-number}
+locations using a @var{breakpoint-number} and two @var{location-number}s,
+in increasing order, separated by a hyphen, like
+@kbd{@var{breakpoint-number}.@var{location-number1}-@var{location-number2}},
+in which case @value{GDBN} acts on all the locations in the range (inclusive).
+Disabling or enabling the parent breakpoint (@pxref{Disabling}) affects
+all of the locations that belong to that breakpoint.
 
 @cindex pending breakpoints
 It's quite common to have a breakpoint inside a shared library.
diff --git a/gdb/NEWS b/gdb/NEWS
index fbf5591..aadb7a3 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -75,6 +75,9 @@ QSetWorkingDir
 * The "maintenance selftest" command now takes an optional argument to
   filter the tests to be run.
 
+* The "enable", and "disable" commands now accept a range of
+  breakpoint locations, e.g. "enable 1.3-5".
+
 * New commands
 
 set|show cwd
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ada72e0..2c5a3a4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14161,59 +14161,66 @@ ignore_command (char *args, int from_tty)
     printf_filtered ("\n");
 }
 \f
-/* Call FUNCTION on each of the breakpoints
-   whose numbers are given in ARGS.  */
+
+/* Call FUNCTION on each of the breakpoints with numbers in the range
+   defined by BP_NUM_RANGE (an inclusive range).  */
 
 static void
-map_breakpoint_numbers (const char *args,
-			gdb::function_view<void (breakpoint *)> function)
+map_breakpoint_number_range (std::pair<int, int> bp_num_range,
+			     gdb::function_view<void (breakpoint *)> function)
 {
-  int num;
-  struct breakpoint *b, *tmp;
-
-  if (args == 0 || *args == '\0')
-    error_no_arg (_("one or more breakpoint numbers"));
-
-  number_or_range_parser parser (args);
-
-  while (!parser.finished ())
+  if (bp_num_range.first == 0)
+    {
+      warning (_("bad breakpoint number at or near '%d'"),
+	       bp_num_range.first);
+    }
+  else
     {
-      const char *p = parser.cur_tok ();
-      bool match = false;
+      struct breakpoint *b, *tmp;
 
-      num = parser.get_number ();
-      if (num == 0)
-	{
-	  warning (_("bad breakpoint number at or near '%s'"), p);
-	}
-      else
+      for (int i = bp_num_range.first; i <= bp_num_range.second; i++)
 	{
+	  bool match = false;
+
 	  ALL_BREAKPOINTS_SAFE (b, tmp)
-	    if (b->number == num)
+	    if (b->number == i)
 	      {
 		match = true;
 		function (b);
 		break;
 	      }
 	  if (!match)
-	    printf_unfiltered (_("No breakpoint number %d.\n"), num);
+	    printf_unfiltered (_("No breakpoint number %d.\n"), i);
 	}
     }
 }
 
+/* Call FUNCTION on each of the breakpoints whose numbers are given in
+   ARGS.  */
+
+static void
+map_breakpoint_numbers (const char *args,
+			gdb::function_view<void (breakpoint *)> function)
+{
+  if (args == NULL || *args == '\0')
+    error_no_arg (_("one or more breakpoint numbers"));
+
+  number_or_range_parser parser (args);
+
+  while (!parser.finished ())
+    {
+      int num = parser.get_number ();
+      map_breakpoint_number_range (std::make_pair (num, num), function);
+    }
+}
+
+/* Return the breakpoint location structure corresponding to the
+   BP_NUM and LOC_NUM values.  */
+
 static struct bp_location *
-find_location_by_number (const char *number)
+find_location_by_number (int bp_num, int loc_num)
 {
-  const char *p1;
-  int bp_num;
-  int loc_num;
   struct breakpoint *b;
-  struct bp_location *loc;  
-
-  p1 = number;
-  bp_num = get_number_trailer (&p1, '.');
-  if (bp_num == 0 || p1[0] != '.')
-    error (_("Bad breakpoint number '%s'"), number);
 
   ALL_BREAKPOINTS (b)
     if (b->number == bp_num)
@@ -14222,25 +14229,153 @@ find_location_by_number (const char *number)
       }
 
   if (!b || b->number != bp_num)
-    error (_("Bad breakpoint number '%s'"), number);
+    error (_("Bad breakpoint number '%d'"), bp_num);
   
-  /* Skip the dot.  */
-  ++p1;
-  const char *save = p1;
-  loc_num = get_number (&p1);
   if (loc_num == 0)
-    error (_("Bad breakpoint location number '%s'"), number);
+    error (_("Bad breakpoint location number '%d'"), loc_num);
 
-  --loc_num;
-  loc = b->loc;
-  for (;loc_num && loc; --loc_num, loc = loc->next)
-    ;
-  if (!loc)
-    error (_("Bad breakpoint location number '%s'"), save);
-    
-  return loc;  
+  int n = 0;
+  for (bp_location *loc = b->loc; loc != NULL; loc = loc->next)
+    if (++n == loc_num)
+      return loc;
+
+  error (_("Bad breakpoint location number '%d'"), loc_num);
 }
 
+/* Extract the breakpoint/location range specified by ARG.  Returns
+   the breakpoint range in BP_NUM_RANGE, and the location range in
+   BP_LOC_RANGE.
+
+   ARG may be in any of the following forms:
+
+   x     where 'x' is a breakpoint number.
+   x-y   where 'x' and 'y' specify a breakpoint numbers range.
+   x.y   where 'x' is a breakpoint number and 'z' a location number.
+   x.y-z where 'x' is a breakpoint number and 'y' and 'z' specify a
+	 location number range.
+*/
+
+static bool
+extract_bp_number_and_location (const std::string &arg,
+				std::pair<int, int> &bp_num_range,
+				std::pair<int, int> &bp_loc_range)
+{
+  std::string::size_type dot = arg.find ('.');
+
+  if (dot != std::string::npos)
+    {
+      /* Handle 'x.y' and 'x.y-z' cases.  */
+
+      if (arg.length () == dot + 1 || dot == 0)
+	error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
+
+      const char *ptb = arg.c_str ();
+      int bp_num = get_number_trailer (&ptb, '.');
+      if (bp_num == 0)
+	error (_("Bad breakpoint number '%s'"), arg.c_str ());
+
+      bp_num_range.first = bp_num;
+      bp_num_range.second = bp_num;
+
+      const char *bp_loc = &arg[dot + 1];
+      std::string::size_type dash = arg.find ('-', dot + 1);
+      if (dash != std::string::npos)
+	{
+	  /* bp_loc is a range (x-z).  */
+	  if (arg.length () == dash + 1)
+	    error (_("bad breakpoint number at or near: '%s'"), bp_loc);
+
+	  const char *ptlf = bp_loc;
+	  bp_loc_range.first = get_number_trailer (&ptlf, '-');
+
+	  const char *ptls = &arg[dash + 1];
+	  bp_loc_range.second = get_number_trailer (&ptls, '\0');
+	}
+      else
+	{
+	  /* bp_loc is a single value.  */
+	  const char *ptls = bp_loc;
+	  bp_loc_range.first = get_number_trailer (&ptls, '\0');
+	  if (bp_loc_range.first == 0)
+	    {
+	      warning (_("bad breakpoint number at or near '%s'"), arg.c_str ());
+	      return false;
+	    }
+	  bp_loc_range.second = bp_loc_range.first;
+	}
+    }
+  else
+    {
+      /* Handle x and x-y cases.  */
+      std::string::size_type dash = arg.find ('-');
+      if (dash != std::string::npos)
+	{
+	  if (arg.length () == dash + 1 || dash == 0)
+	    error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
+
+	  const char *ptf = arg.c_str ();
+	  bp_num_range.first = get_number_trailer (&ptf, '-');
+
+	  const char *pts = &arg[dash + 1];
+	  bp_num_range.second = get_number_trailer (&pts, '\0');
+	}
+      else
+	{
+	  const char *ptf = arg.c_str ();
+	  bp_num_range.first = get_number (&ptf);
+	  if (bp_num_range.first == 0)
+	    {
+	      warning (_("bad breakpoint number at or near '%s'"), arg.c_str ());
+	      return false;
+	    }
+	  bp_num_range.second = bp_num_range.first;
+	}
+      bp_loc_range.first = 0;
+      bp_loc_range.second = 0;
+    }
+
+  if (bp_num_range.first == 0 || bp_num_range.second == 0)
+    error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
+
+  return true;
+}
+
+/* Enable or disable a breakpoint location BP_NUM.LOC_NUM.  ENABLE
+   specifies whether to enable or disable.  */
+
+static void
+enable_disable_bp_num_loc (int bp_num, int loc_num, bool enable)
+{
+  struct bp_location *loc = find_location_by_number (bp_num, loc_num);
+  if (loc != NULL)
+    {
+      if (loc->enabled != enable)
+	{
+	  loc->enabled = enable;
+	  mark_breakpoint_location_modified (loc);
+	}
+      if (target_supports_enable_disable_tracepoint ()
+	  && current_trace_status ()->running && loc->owner
+	  && is_tracepoint (loc->owner))
+	target_disable_tracepoint (loc);
+    }
+  update_global_location_list (UGLL_DONT_INSERT);
+}
+
+/* Enable or disable a range of breakpoint locations.  BP_NUM is the
+   number of the breakpoint, and BP_LOC_RANGE specifies the
+   (inclusive) range of location numbers of that breakpoint to
+   enable/disable.  ENABLE specifies whether to enable or disable the
+   location.  */
+
+static void
+enable_disable_breakpoint_location_range (int bp_num,
+					  std::pair<int, int> &bp_loc_range,
+					  bool enable)
+{
+  for (int i = bp_loc_range.first; i <= bp_loc_range.second; i++)
+    enable_disable_bp_num_loc (bp_num, i, enable);
+}
 
 /* Set ignore-count of breakpoint number BPTNUM to COUNT.
    If from_tty is nonzero, it prints a message to that effect,
@@ -14274,8 +14409,13 @@ disable_breakpoint (struct breakpoint *bpt)
   observer_notify_breakpoint_modified (bpt);
 }
 
+/* Enable or disable the breakpoint(s) or breakpoint location(s)
+   specified in ARGS.  ARGS may be in any of the formats handled by
+   extract_bp_number_and_location.  ENABLE specifies whether to enable
+   or disable the breakpoints/locations.  */
+
 static void
-disable_command (const char *args, int from_tty)
+enable_disable_command (const char *args, int from_tty, bool enable)
 {
   if (args == 0)
     {
@@ -14283,7 +14423,12 @@ disable_command (const char *args, int from_tty)
 
       ALL_BREAKPOINTS (bpt)
 	if (user_breakpoint_p (bpt))
-	  disable_breakpoint (bpt);
+	  {
+	    if (enable)
+	      enable_breakpoint (bpt);
+	    else
+	      disable_breakpoint (bpt);
+	  }
     }
   else
     {
@@ -14291,35 +14436,43 @@ disable_command (const char *args, int from_tty)
 
       while (!num.empty ())
 	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
+	  std::pair<int, int> bp_num_range, bp_loc_range;
 
-	      if (loc)
+	  if (extract_bp_number_and_location (num, bp_num_range, bp_loc_range))
+	    {
+	      if (bp_loc_range.first == bp_loc_range.second
+		  && bp_loc_range.first == 0)
 		{
-		  if (loc->enabled)
-		    {
-		      loc->enabled = 0;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_disable_tracepoint (loc);
+		  /* Handle breakpoint ids with formats 'x' or 'x-z'.  */
+		  map_breakpoint_number_range (bp_num_range,
+					       enable
+					       ? enable_breakpoint
+					       : disable_breakpoint);
+		}
+	      else
+		{
+		  /* Handle breakpoint ids with formats 'x.y' or
+		     'x.y-z'.  */
+		  enable_disable_breakpoint_location_range
+		    (bp_num_range.first, bp_loc_range, enable);
 		}
-	      update_global_location_list (UGLL_DONT_INSERT);
 	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, disable_breakpoint);
-	       });
 	  num = extract_arg (&args);
 	}
     }
 }
 
+/* The disable command disables the specified breakpoints/locations
+   (or all defined breakpoints) so they're no longer effective in
+   stopping the inferior.  ARGS may be in any of the forms defined in
+   extract_bp_number_and_location.  */
+
+static void
+disable_command (const char *args, int from_tty)
+{
+  enable_disable_command (args, from_tty, false);
+}
+
 static void
 enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
 			int count)
@@ -14390,54 +14543,15 @@ enable_breakpoint (struct breakpoint *bpt)
   enable_breakpoint_disp (bpt, bpt->disposition, 0);
 }
 
-/* The enable command enables the specified breakpoints (or all defined
-   breakpoints) so they once again become (or continue to be) effective
-   in stopping the inferior.  */
+/* The enable command enables the specified breakpoints/locations (or
+   all defined breakpoints) so they once again become (or continue to
+   be) effective in stopping the inferior.  ARGS may be in any of the
+   forms defined in extract_bp_number_and_location.  */
 
 static void
 enable_command (const char *args, int from_tty)
 {
-  if (args == 0)
-    {
-      struct breakpoint *bpt;
-
-      ALL_BREAKPOINTS (bpt)
-	if (user_breakpoint_p (bpt))
-	  enable_breakpoint (bpt);
-    }
-  else
-    {
-      std::string num = extract_arg (&args);
-
-      while (!num.empty ())
-	{
-	  if (num.find ('.') != std::string::npos)
-	    {
-	      struct bp_location *loc = find_location_by_number (num.c_str ());
-
-	      if (loc)
-		{
-		  if (!loc->enabled)
-		    {
-		      loc->enabled = 1;
-		      mark_breakpoint_location_modified (loc);
-		    }
-		  if (target_supports_enable_disable_tracepoint ()
-		      && current_trace_status ()->running && loc->owner
-		      && is_tracepoint (loc->owner))
-		    target_enable_tracepoint (loc);
-		}
-	      update_global_location_list (UGLL_MAY_INSERT);
-	    }
-	  else
-	    map_breakpoint_numbers
-	      (num.c_str (), [&] (breakpoint *b)
-	       {
-		 iterate_over_related_breakpoints (b, enable_breakpoint);
-	       });
-	  num = extract_arg (&args);
-	}
-    }
+  enable_disable_command (args, from_tty, true);
 }
 
 static void
diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
index d407408..9b8d251 100644
--- a/gdb/testsuite/gdb.base/ena-dis-br.exp
+++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
@@ -324,6 +324,9 @@ set b4 [break_at main ""]
 #
 # WHAT - the command to test (disable/enable).
 #
+# Note: tests involving location ranges (and more) are found in
+# gdb.cp/ena-dis-br-range.exp.
+#
 proc test_ena_dis_br { what } {
     global b1
     global b2
diff --git a/gdb/testsuite/gdb.cp/ena-dis-br-range.cc b/gdb/testsuite/gdb.cp/ena-dis-br-range.cc
new file mode 100644
index 0000000..9469e34
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/ena-dis-br-range.cc
@@ -0,0 +1,66 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Some overloaded functions to test breakpoints with multiple
+   locations.  */
+
+class foo
+{
+public:
+  static void overload (void);
+  static void overload (char);
+  static void overload (int);
+  static void overload (double);
+};
+
+void
+foo::overload ()
+{
+}
+
+void
+foo::overload (char arg)
+{
+}
+
+void
+foo::overload (int arg)
+{
+}
+
+void
+foo::overload (double arg)
+{
+}
+
+void
+marker ()
+{
+}
+
+int
+main ()
+{
+  foo::overload ();
+  foo::overload (111);
+  foo::overload ('h');
+  foo::overload (3.14);
+
+  marker ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
new file mode 100644
index 0000000..a17ee46
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
@@ -0,0 +1,134 @@
+# Copyright 2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test the enable/disable commands with breakpoint location ranges.
+
+# Note: more tests involving involving disable/enable commands on
+# multiple locations and breakpoints are found in
+# gdb.base/ena-dis-br.exp.
+
+if { [skip_cplus_tests] } { continue }
+
+standard_testfile .cc
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
+    return -1
+}
+
+# Set it up at a breakpoint so we can play with the variable values.
+
+if ![runto 'marker'] then {
+    fail "run to marker"
+    return -1
+}
+
+# Returns a buffer corresponding to what GDB replies when asking for
+# 'info breakpoint'.  The parameters are all the existing breakpoints
+# enabled/disable value: 'n' or 'y'.
+
+proc make_info_breakpoint_reply_re {b1 b2 b21 b22 b23 b24} {
+    set ws "\[\t \]+"
+    return [multi_line \
+		"Num     Type${ws}Disp Enb Address${ws}What.*" \
+		"1${ws}breakpoint     keep ${b1}${ws}.* in marker\\(\\) at .*" \
+		"${ws}breakpoint already hit 1 time.*" \
+		"2${ws}breakpoint${ws}keep${ws}${b2}${ws}<MULTIPLE>.*" \
+		"2.1${ws}${b21}.*" \
+		"2.2${ws}${b22}.*" \
+		"2.3${ws}${b23}.*" \
+		"2.4${ws}${b24}.*" \
+	       ]
+}
+
+gdb_test "break foo::overload" \
+    "Breakpoint \[0-9\]+ at $hex: foo::overload. .4 locations." \
+    "set breakpoint at overload"
+
+gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
+    "breakpoint info"
+
+# Test the enable/disable commands, and check the enable/disable state
+# of the breakpoints/locations in the "info break" output.  CMD is the
+# actual disable/enable command. The bNN parameters are the same as
+# make_info_breakpoint_reply_re's.
+proc test_enable_disable {cmd b1 b2 b21 b22 b23 b24} {
+    gdb_test_no_output $cmd
+
+    set re [make_info_breakpoint_reply_re $b1 $b2 $b21 $b22 $b23 $b24]
+    gdb_test "info break" $re "breakpoint info $cmd"
+}
+
+# Check that we can disable/enable a breakpoint with a single
+# location.
+test_enable_disable "disable 1" n y y y y y
+test_enable_disable "enable  1" y y y y y y
+
+# Check that we can disable/disable a breakpoint with multiple
+# locations.
+test_enable_disable "disable 2" y n y y y y
+test_enable_disable "enable  2" y y y y y y
+
+# Check that we can disable/enable a single location breakpoint.
+test_enable_disable "disable 2.2" y y y n y y
+test_enable_disable "enable  2.2" y y y y y y
+
+# Check that we can disable/enable a range of breakpoint locations.
+test_enable_disable "disable 2.2-3" y y y n n y
+test_enable_disable "enable  2.2-3" y y y y y y
+
+# Check that we can disable/enable a breakpoint location range with
+# START==END.
+test_enable_disable "disable 2.2-2" y y y n y y
+test_enable_disable "enable  2.2-2" y y y y y y
+
+# Check that we can disable a location breakpoint range with max >
+# existing breakpoint location.
+gdb_test "disable 2.3-5" "Bad breakpoint location number '5'" \
+    "disable location breakpoint range with max > existing"
+
+gdb_test "info break" [make_info_breakpoint_reply_re y y y y n n] \
+    "breakpoint info disable 2.3 to 2.5"
+
+# Check that we can enable a location breakpoint range with max >
+# existing breakpoint location.
+gdb_test "enable 2.3-5" "Bad breakpoint location number '5'" \
+    "enable location breakpoint range with max > existing"
+
+gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
+    "breakpoint info enable 2.3 to 2.5"
+
+# Check that disabling an reverse location breakpoint range does not
+# work.
+gdb_test_no_output "disable 2.3-2"
+
+gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
+    "breakpoint info disable 2.3-2"
+
+# Check that disabling an invalid breakpoint location range does not
+# cause unexpected behavior.
+gdb_test "disable 2.6-7" "Bad breakpoint location number '6'" \
+    "disable an unvalid location breakpoint range"
+
+gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
+    "breakpoint info disable 2.6-7"
+
+# Check that disabling an invalid breakpoint location range does not
+# cause trouble.
+gdb_test_no_output "disable 2.8-6"
+
+gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
+    "breakpoint info disable 2.8-6"
-- 
2.5.5

[PATCH v5 4/4] Make breakpoint/location number parsing error output consistent

[Pedro Alves]

... and also make GDB catch a few more cases of invalid input.

This fixes the inconsistency in GDB's output (e.g., "bad" vs "Bad")
exposed by the new tests added in the previous commit.

Also, makes the "0-0" and "inverted range" cases be loud errors.

Also makes GDB reject negative breakpoint number in ranges.  We
already rejected negative number literals, but you could still subvert
that via convenience variables, like:

  (gdb) set $bp -1
  (gdb) disable $bp.1-2

The change to get_number_trailer fixes a bug exposed by the new tests.
The function did not handle parsing "-$num".  [This wasn't visible in
the gdb.multi/tids.exp (which has similar tests) because the TID range
parsing is implemented differently.]

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (extract_bp_kind): New enum.
	(extract_bp_num, extract_bp_or_bp_range): New functions, partially
	factored out from ...
	(extract_bp_number_and_location): ... here.
	* cli/cli-utils.c (get_number_trailer): Handle '-$variable'.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/ena-dis-br.exp (test_ena_dis_br): Adjust test.
	* gdb.cp/ena-dis-br-range.exp: Adjust tests.
	(disable_invalid, disable_inverted, disable_negative): New
	procedures.
	("bad numbers"): New set of tests.
---
 gdb/breakpoint.c                          | 141 +++++++++++++++++++-----------
 gdb/cli/cli-utils.c                       |  16 ++--
 gdb/testsuite/gdb.base/ena-dis-br.exp     |   2 +-
 gdb/testsuite/gdb.cp/ena-dis-br-range.exp | 115 +++++++++++++++++++-----
 4 files changed, 191 insertions(+), 83 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 43a7d87..ebf28de 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14242,6 +14242,87 @@ find_location_by_number (int bp_num, int loc_num)
   error (_("Bad breakpoint location number '%d'"), loc_num);
 }
 
+/* Modes of operation for extract_bp_num.  */
+enum class extract_bp_kind
+{
+  /* Extracting a breakpoint number.  */
+  bp,
+
+  /* Extracting a location number.  */
+  loc,
+};
+
+/* Extract a breakpoint or location number (as determined by KIND)
+   from the string starting at START.  TRAILER is a character which
+   can be found after the number.  If you don't want a trailer, use
+   '\0'.  If END_OUT is not NULL, it is set to point after the parsed
+   string.  This always returns a positive integer.  */
+
+static int
+extract_bp_num (extract_bp_kind kind, const char *start,
+		int trailer, const char **end_out = NULL)
+{
+  const char *end = start;
+  int num = get_number_trailer (&end, trailer);
+  if (num < 0)
+    error (kind == extract_bp_kind::bp
+	   ? _("Negative breakpoint number '%.*s'")
+	   : _("Negative breakpoint location number '%.*s'"),
+	   int (end - start), start);
+  if (num == 0)
+    error (kind == extract_bp_kind::bp
+	   ? _("Bad breakpoint number '%.*s'")
+	   : _("Bad breakpoint location number '%.*s'"),
+	   int (end - start), start);
+
+  if (end_out != NULL)
+    *end_out = end;
+  return num;
+}
+
+/* Extract a breakpoint or location range (as determined by KIND) in
+   the form NUM1-NUM2 stored at &ARG[arg_offset].  Returns a std::pair
+   representing the (inclusive) range.  The returned pair's elements
+   are always positive integers.  */
+
+static std::pair<int, int>
+extract_bp_or_bp_range (extract_bp_kind kind,
+			const std::string &arg,
+			std::string::size_type arg_offset)
+{
+  std::pair<int, int> range;
+  const char *bp_loc = &arg[arg_offset];
+  std::string::size_type dash = arg.find ('-', arg_offset);
+  if (dash != std::string::npos)
+    {
+      /* bp_loc is a range (x-z).  */
+      if (arg.length () == dash + 1)
+	error (kind == extract_bp_kind::bp
+	       ? _("Bad breakpoint number at or near: '%s'")
+	       : _("Bad breakpoint location number at or near: '%s'"),
+	       bp_loc);
+
+      const char *end;
+      const char *start_first = bp_loc;
+      const char *start_second = &arg[dash + 1];
+      range.first = extract_bp_num (kind, start_first, '-');
+      range.second = extract_bp_num (kind, start_second, '\0', &end);
+
+      if (range.first > range.second)
+	error (kind == extract_bp_kind::bp
+	       ? _("Inverted breakpoint range at '%.*s'")
+	       : _("Inverted breakpoint location range at '%.*s'"),
+	       int (end - start_first), start_first);
+    }
+  else
+    {
+      /* bp_loc is a single value.  */
+      range.first = extract_bp_num (kind, bp_loc, '\0');
+      range.second = range.first;
+    }
+  return range;
+}
+
 /* Extract the breakpoint/location range specified by ARG.  Returns
    the breakpoint range in BP_NUM_RANGE, and the location range in
    BP_LOC_RANGE.
@@ -14267,69 +14348,23 @@ extract_bp_number_and_location (const std::string &arg,
       /* Handle 'x.y' and 'x.y-z' cases.  */
 
       if (arg.length () == dot + 1 || dot == 0)
-	error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
-
-      const char *ptb = arg.c_str ();
-      int bp_num = get_number_trailer (&ptb, '.');
-      if (bp_num == 0)
-	error (_("Bad breakpoint number '%s'"), arg.c_str ());
+	error (_("Bad breakpoint number at or near: '%s'"), arg.c_str ());
 
-      bp_num_range.first = bp_num;
-      bp_num_range.second = bp_num;
+      bp_num_range.first
+	= extract_bp_num (extract_bp_kind::bp, arg.c_str (), '.');
+      bp_num_range.second = bp_num_range.first;
 
-      const char *bp_loc = &arg[dot + 1];
-      std::string::size_type dash = arg.find ('-', dot + 1);
-      if (dash != std::string::npos)
-	{
-	  /* bp_loc is a range (x-z).  */
-	  if (arg.length () == dash + 1)
-	    error (_("bad breakpoint number at or near: '%s'"), bp_loc);
-
-	  const char *ptlf = bp_loc;
-	  bp_loc_range.first = get_number_trailer (&ptlf, '-');
-
-	  const char *ptls = &arg[dash + 1];
-	  bp_loc_range.second = get_number_trailer (&ptls, '\0');
-	}
-      else
-	{
-	  /* bp_loc is a single value.  */
-	  const char *ptls = bp_loc;
-	  bp_loc_range.first = get_number_trailer (&ptls, '\0');
-	  if (bp_loc_range.first == 0)
-	    error (_("bad breakpoint number at or near '%s'"), arg.c_str ());
-	  bp_loc_range.second = bp_loc_range.first;
-	}
+      bp_loc_range = extract_bp_or_bp_range (extract_bp_kind::loc,
+					     arg, dot + 1);
     }
   else
     {
       /* Handle x and x-y cases.  */
-      std::string::size_type dash = arg.find ('-');
-      if (dash != std::string::npos)
-	{
-	  if (arg.length () == dash + 1 || dash == 0)
-	    error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
-
-	  const char *ptf = arg.c_str ();
-	  bp_num_range.first = get_number_trailer (&ptf, '-');
 
-	  const char *pts = &arg[dash + 1];
-	  bp_num_range.second = get_number_trailer (&pts, '\0');
-	}
-      else
-	{
-	  const char *ptf = arg.c_str ();
-	  bp_num_range.first = get_number (&ptf);
-	  if (bp_num_range.first == 0)
-	    error (_("bad breakpoint number at or near '%s'"), arg.c_str ());
-	  bp_num_range.second = bp_num_range.first;
-	}
+      bp_num_range = extract_bp_or_bp_range (extract_bp_kind::bp, arg, 0);
       bp_loc_range.first = 0;
       bp_loc_range.second = 0;
     }
-
-  if (bp_num_range.first == 0 || bp_num_range.second == 0)
-    error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
 }
 
 /* Enable or disable a breakpoint location BP_NUM.LOC_NUM.  ENABLE
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index d5273b5..6b36378 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -30,6 +30,13 @@ get_number_trailer (const char **pp, int trailer)
 {
   int retval = 0;	/* default */
   const char *p = *pp;
+  bool negative = false;
+
+  if (*p == '-')
+    {
+      ++p;
+      negative = true;
+    }
 
   if (*p == '$')
     {
@@ -70,11 +77,10 @@ get_number_trailer (const char **pp, int trailer)
     }
   else
     {
-      if (*p == '-')
-	++p;
+      const char *p1 = p;
       while (*p >= '0' && *p <= '9')
 	++p;
-      if (p == *pp)
+      if (p == p1)
 	/* There is no number here.  (e.g. "cond a == b").  */
 	{
 	  /* Skip non-numeric token.  */
@@ -84,7 +90,7 @@ get_number_trailer (const char **pp, int trailer)
 	  retval = 0;
 	}
       else
-	retval = atoi (*pp);
+	retval = atoi (p1);
     }
   if (!(isspace (*p) || *p == '\0' || *p == trailer))
     {
@@ -95,7 +101,7 @@ get_number_trailer (const char **pp, int trailer)
     }
   p = skip_spaces (p);
   *pp = p;
-  return retval;
+  return negative ? -retval : retval;
 }
 
 /* See documentation in cli-utils.h.  */
diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
index 2bbb734..8655421 100644
--- a/gdb/testsuite/gdb.base/ena-dis-br.exp
+++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
@@ -398,7 +398,7 @@ proc test_ena_dis_br { what } {
     # Now enable(disable) '$b4.1 fooobaar'.  This should error on
     # fooobaar.
     gdb_test "$what $b4.1 fooobaar" \
-       "bad breakpoint number at or near 'fooobaar'" \
+       "Bad breakpoint number 'fooobaar'" \
        "$what \$b4.1 fooobar"
     set test1 "${what}d \$b4.1"
 
diff --git a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
index 0ebc45f..629b0bc 100644
--- a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
+++ b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
@@ -111,9 +111,9 @@ gdb_test "enable 2.3-5" "Bad breakpoint location number '5'" \
 gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
     "breakpoint info enable 2.3 to 2.5"
 
-# Check that disabling an reverse location breakpoint range does not
+# Check that disabling an inverted breakpoint location range does not
 # work.
-gdb_test_no_output "disable 2.3-2"
+gdb_test "disable 2.3-2" "Inverted breakpoint location range at '3-2'"
 
 gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
     "breakpoint info disable 2.3-2"
@@ -128,44 +128,111 @@ gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
 
 # Check that disabling an invalid breakpoint location range does not
 # cause trouble.
-gdb_test_no_output "disable 2.8-6"
+gdb_test "disable 2.8-6" "Inverted breakpoint location range at '8-6'"
 
 gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
     "breakpoint info disable 2.8-6"
 
 # Check that invalid/open ranges are handled correctly.
 with_test_prefix "open range" {
-    gdb_test "disable -" "bad breakpoint number at or near: '-'"
-    gdb_test "disable -1" "bad breakpoint number at or near: '-1'"
-    gdb_test "disable 1-" "bad breakpoint number at or near: '1-'"
-    gdb_test "disable 1.-2" "Bad breakpoint location number '-2'"
-    gdb_test "disable 1.2-" "bad breakpoint number at or near: '2-'"
-    gdb_test "disable 1.-2-3" "Bad breakpoint location number '-2'"
-    gdb_test "disable 1-2-3" "bad breakpoint number at or near: '1-2-3'"
+    gdb_test "disable -" "Bad breakpoint number at or near: '-'"
+    gdb_test "disable -1" "Negative breakpoint number '-1'"
+    gdb_test "disable 1-" "Bad breakpoint number at or near: '1-'"
+    gdb_test "disable 1.-2" "Negative breakpoint location number '-2'"
+    gdb_test "disable 1.2-" "Bad breakpoint location number at or near: '2-'"
+    gdb_test "disable 1.-2-3" "Negative breakpoint location number '-2'"
+    gdb_test "disable 1-2-3" "Bad breakpoint number '2-3'"
 }
 
 with_test_prefix "dangling period" {
-    gdb_test "disable 2." "bad breakpoint number at or near: '2.'"
-    gdb_test "disable .2" "bad breakpoint number at or near: '.2'"
-    gdb_test "disable 2.3.4" "bad breakpoint number at or near '2.3.4'"
+    gdb_test "disable 2." "Bad breakpoint number at or near: '2.'"
+    gdb_test "disable .2" "Bad breakpoint number at or near: '.2'"
+    gdb_test "disable 2.3.4" "Bad breakpoint location number '3.4'"
 }
 
 # Check that 0s are handled correctly.
 with_test_prefix "zero" {
-    gdb_test "disable 0" "bad breakpoint number at or near '0'"
-    gdb_test "disable 0.0" "Bad breakpoint number '0.0'"
-    gdb_test "disable 0.1" "Bad breakpoint number '0.1'"
-    gdb_test "disable 0.1-2" "Bad breakpoint number '0.1-2'"
-    gdb_test "disable 2.0" "bad breakpoint number at or near '2.0'"
+    gdb_test "disable 0" "Bad breakpoint number '0'"
+    gdb_test "disable 0.0" "Bad breakpoint number '0'"
+    gdb_test "disable 0.1" "Bad breakpoint number '0'"
+    gdb_test "disable 0.1-2" "Bad breakpoint number '0'"
+    gdb_test "disable 2.0" "Bad breakpoint location number '0'"
+    gdb_test "disable 2.0-0" "Bad breakpoint location number '0'"
+    gdb_test "disable 2.0-1" "Bad breakpoint location number '0'"
+    gdb_test "disable 2.1-0" "Bad breakpoint location number '0'"
+}
+
+# Test "disable BPLIST" with an invalid breakpoint/location BPLIST.
+# PREFIX and SUFFIX are concatenated to form BPLIST.  The invalid part
+# is always in SUFFIX.
+
+proc disable_invalid {prefix suffix} {
+    set bad_re [string_to_regexp $suffix]
+
+    if {$prefix == ""} {
+	gdb_test \
+	    "disable $suffix" \
+	    "Bad breakpoint number '${bad_re}'"
+    } else {
+	gdb_test \
+	    "disable ${prefix}$suffix" \
+	    "Bad breakpoint location number '${bad_re}'"
+    }
+}
 
-    # These should really fail...
-    gdb_test_no_output "disable 2.0-0"
-    gdb_test_no_output "enable 2.0-0"
+# Like disable_invalid, but expects an "inverted range" error.
 
-    gdb_test "disable 2.0-1" "Bad breakpoint location number '0'"
+proc disable_inverted {prefix suffix} {
+    set bad_re [string_to_regexp $suffix]
+
+    if {$prefix == ""} {
+	gdb_test \
+	    "disable $suffix" \
+	    "Inverted breakpoint range at '${bad_re}'"
+    } else {
+	gdb_test \
+	    "disable ${prefix}$suffix" \
+	    "Inverted breakpoint location range at '${bad_re}'"
+    }
+}
+
+# Like disable_invalid, but expects a "negative number" error.
+
+proc disable_negative {prefix suffix} {
+    set bad_re [string_to_regexp $suffix]
+
+    if {$prefix == ""} {
+	gdb_test \
+	    "disable $suffix" \
+	    "Negative breakpoint number '${bad_re}'"
+    } else {
+	gdb_test \
+	    "disable ${prefix}$suffix" \
+	    "Negative breakpoint location number '${bad_re}'"
+    }
+}
 
-    # Likewise, should fail.
-    gdb_test_no_output "disable 2.1-0"
+with_test_prefix "bad numbers" {
+    gdb_test "p \$zero = 0" " = 0"
+    gdb_test "p \$one = 1" " = 1"
+    gdb_test "p \$two = 2" " = 2"
+    gdb_test "p \$minus_one = -1" " = -1"
+    foreach prefix {"" "1." "$one."} {
+	set prefix_re [string_to_regexp $prefix]
+
+	disable_invalid $prefix "foo"
+	disable_invalid $prefix "1foo"
+	disable_invalid $prefix "foo1"
+
+	disable_inverted $prefix "2-1"
+	disable_inverted $prefix "2-\$one"
+	disable_inverted $prefix "\$two-1"
+	disable_inverted $prefix "\$two-\$one"
+
+	disable_negative $prefix "-1"
+	disable_negative $prefix "-\$one"
+	disable_negative $prefix "\$minus_one"
+    }
 }
 
 gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
-- 
2.5.5

[PATCH v5 2/4] Breakpoint location parsing: always error instead of warning

[Pedro Alves]

It's odd that when parsing a breakpoint or location number, we error out
in most cases, but warn in others.

  (gdb) disable 1-
  bad breakpoint number at or near: '1-'
  (gdb) disable -1
  bad breakpoint number at or near: '-1'
  (gdb) disable .foo
  bad breakpoint number at or near: '.foo'
  (gdb) disable foo.1
  Bad breakpoint number 'foo.1'
  (gdb) disable 1.foo
  warning: bad breakpoint number at or near '1.foo'

This changes GDB to always error out.  It required touching one testcase
that expected the warning.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (extract_bp_number_and_location): Change return
	type to void.  Throw error instead of warning.
	(enable_disable_command): Adjust.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.base/ena-dis-br.exp: Don't expect "warning:".
---
 gdb/breakpoint.c                      | 47 ++++++++++++++---------------------
 gdb/testsuite/gdb.base/ena-dis-br.exp |  6 ++---
 2 files changed, 22 insertions(+), 31 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 2c5a3a4..43a7d87 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14255,7 +14255,7 @@ find_location_by_number (int bp_num, int loc_num)
 	 location number range.
 */
 
-static bool
+static void
 extract_bp_number_and_location (const std::string &arg,
 				std::pair<int, int> &bp_num_range,
 				std::pair<int, int> &bp_loc_range)
@@ -14297,10 +14297,7 @@ extract_bp_number_and_location (const std::string &arg,
 	  const char *ptls = bp_loc;
 	  bp_loc_range.first = get_number_trailer (&ptls, '\0');
 	  if (bp_loc_range.first == 0)
-	    {
-	      warning (_("bad breakpoint number at or near '%s'"), arg.c_str ());
-	      return false;
-	    }
+	    error (_("bad breakpoint number at or near '%s'"), arg.c_str ());
 	  bp_loc_range.second = bp_loc_range.first;
 	}
     }
@@ -14324,10 +14321,7 @@ extract_bp_number_and_location (const std::string &arg,
 	  const char *ptf = arg.c_str ();
 	  bp_num_range.first = get_number (&ptf);
 	  if (bp_num_range.first == 0)
-	    {
-	      warning (_("bad breakpoint number at or near '%s'"), arg.c_str ());
-	      return false;
-	    }
+	    error (_("bad breakpoint number at or near '%s'"), arg.c_str ());
 	  bp_num_range.second = bp_num_range.first;
 	}
       bp_loc_range.first = 0;
@@ -14336,8 +14330,6 @@ extract_bp_number_and_location (const std::string &arg,
 
   if (bp_num_range.first == 0 || bp_num_range.second == 0)
     error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
-
-  return true;
 }
 
 /* Enable or disable a breakpoint location BP_NUM.LOC_NUM.  ENABLE
@@ -14438,24 +14430,23 @@ enable_disable_command (const char *args, int from_tty, bool enable)
 	{
 	  std::pair<int, int> bp_num_range, bp_loc_range;
 
-	  if (extract_bp_number_and_location (num, bp_num_range, bp_loc_range))
+	  extract_bp_number_and_location (num, bp_num_range, bp_loc_range);
+
+	  if (bp_loc_range.first == bp_loc_range.second
+	      && bp_loc_range.first == 0)
 	    {
-	      if (bp_loc_range.first == bp_loc_range.second
-		  && bp_loc_range.first == 0)
-		{
-		  /* Handle breakpoint ids with formats 'x' or 'x-z'.  */
-		  map_breakpoint_number_range (bp_num_range,
-					       enable
-					       ? enable_breakpoint
-					       : disable_breakpoint);
-		}
-	      else
-		{
-		  /* Handle breakpoint ids with formats 'x.y' or
-		     'x.y-z'.  */
-		  enable_disable_breakpoint_location_range
-		    (bp_num_range.first, bp_loc_range, enable);
-		}
+	      /* Handle breakpoint ids with formats 'x' or 'x-z'.  */
+	      map_breakpoint_number_range (bp_num_range,
+					   enable
+					   ? enable_breakpoint
+					   : disable_breakpoint);
+	    }
+	  else
+	    {
+	      /* Handle breakpoint ids with formats 'x.y' or
+		 'x.y-z'.  */
+	      enable_disable_breakpoint_location_range
+		(bp_num_range.first, bp_loc_range, enable);
 	    }
 	  num = extract_arg (&args);
 	}
diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
index 9b8d251..2bbb734 100644
--- a/gdb/testsuite/gdb.base/ena-dis-br.exp
+++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
@@ -395,10 +395,10 @@ proc test_ena_dis_br { what } {
        }
     }
 
-    # Now enable(disable) $b4.1 fooobaar and
-    # it should give warning on fooobaar.
+    # Now enable(disable) '$b4.1 fooobaar'.  This should error on
+    # fooobaar.
     gdb_test "$what $b4.1 fooobaar" \
-       "warning: bad breakpoint number at or near 'fooobaar'" \
+       "bad breakpoint number at or near 'fooobaar'" \
        "$what \$b4.1 fooobar"
     set test1 "${what}d \$b4.1"
 
-- 
2.5.5

[PATCH v5 0/4] Allow enabling/disabling breakpoint location ranges

[Pedro Alves]

New in v5:

 . Patches #2 -#4 are new.

 . In patch #1:
    - Squashed in Pedro's changes from
      <https://sourceware.org/ml/gdb-patches/2017-10/msg00818.html>,
      and on top of that,
    - Clarified/simplified commit log.
    - Renamed gdb.cp/locbprange.exp -> gdb.cp/ena-dis-br-range.exp to
      mirror the existing gdb.base/ena-dis-br.exp, and added cross
      references between the testcases.

Eli, are the docs in patch #1 OK?

That's patch #1.  makes it possible to enable/disable a range of
breakpoint locations with BPNUM.LOC1-LOC2.

Patches #2-#4 improve GDB's handling of invalid breakpoint/location
numbers and ranges.

Pedro Alves (3):
  Breakpoint location parsing: always error instead of warning
  Add base 'enable/disable invalid location range' tests
  Make breakpoint/location number parsing error output consistent

Xavier Roirand (1):
  Allow enabling/disabling breakpoint location ranges

 gdb/doc/gdb.texinfo                       |  18 +-
 gdb/NEWS                                  |   3 +
 gdb/breakpoint.c                          | 368 +++++++++++++++++++++---------
 gdb/cli/cli-utils.c                       |  16 +-
 gdb/testsuite/gdb.base/ena-dis-br.exp     |   9 +-
 gdb/testsuite/gdb.cp/ena-dis-br-range.cc  |  66 ++++++
 gdb/testsuite/gdb.cp/ena-dis-br-range.exp | 239 +++++++++++++++++++
 7 files changed, 589 insertions(+), 130 deletions(-)
 create mode 100644 gdb/testsuite/gdb.cp/ena-dis-br-range.cc
 create mode 100644 gdb/testsuite/gdb.cp/ena-dis-br-range.exp

-- 
2.5.5

[PATCH v5 3/4] Add base 'enable/disable invalid location range' tests

[Pedro Alves]

This adds tests that exercise the "bad breakpoint number" paths.
Specifically:

 - malformed ranges
 - use of explicit 0 as bp/loc number.
 - inverted ranges

I'm adding this as a baseline to improve.  This shows that there's a
lot of inconsistency in GDB's output (e.g., "bad" vs "Bad").

Also, IMO, the "0-0" and inverted range cases should be loud errors.

That and more will all be addressed in the next patch.

gdb/testsuite/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* gdb.cp/ena-dis-br-range.exp: Add tests.
---
 gdb/testsuite/gdb.cp/ena-dis-br-range.exp | 38 +++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
index a17ee46..0ebc45f 100644
--- a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
+++ b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
@@ -132,3 +132,41 @@ gdb_test_no_output "disable 2.8-6"
 
 gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
     "breakpoint info disable 2.8-6"
+
+# Check that invalid/open ranges are handled correctly.
+with_test_prefix "open range" {
+    gdb_test "disable -" "bad breakpoint number at or near: '-'"
+    gdb_test "disable -1" "bad breakpoint number at or near: '-1'"
+    gdb_test "disable 1-" "bad breakpoint number at or near: '1-'"
+    gdb_test "disable 1.-2" "Bad breakpoint location number '-2'"
+    gdb_test "disable 1.2-" "bad breakpoint number at or near: '2-'"
+    gdb_test "disable 1.-2-3" "Bad breakpoint location number '-2'"
+    gdb_test "disable 1-2-3" "bad breakpoint number at or near: '1-2-3'"
+}
+
+with_test_prefix "dangling period" {
+    gdb_test "disable 2." "bad breakpoint number at or near: '2.'"
+    gdb_test "disable .2" "bad breakpoint number at or near: '.2'"
+    gdb_test "disable 2.3.4" "bad breakpoint number at or near '2.3.4'"
+}
+
+# Check that 0s are handled correctly.
+with_test_prefix "zero" {
+    gdb_test "disable 0" "bad breakpoint number at or near '0'"
+    gdb_test "disable 0.0" "Bad breakpoint number '0.0'"
+    gdb_test "disable 0.1" "Bad breakpoint number '0.1'"
+    gdb_test "disable 0.1-2" "Bad breakpoint number '0.1-2'"
+    gdb_test "disable 2.0" "bad breakpoint number at or near '2.0'"
+
+    # These should really fail...
+    gdb_test_no_output "disable 2.0-0"
+    gdb_test_no_output "enable 2.0-0"
+
+    gdb_test "disable 2.0-1" "Bad breakpoint location number '0'"
+
+    # Likewise, should fail.
+    gdb_test_no_output "disable 2.1-0"
+}
+
+gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
+    "breakpoint info after invalids"
-- 
2.5.5

Re: [PATCH v5 0/4] Allow enabling/disabling breakpoint location ranges

[Eli Zaretskii]

> From: Pedro Alves <palves@redhat.com>
> Date: Thu, 26 Oct 2017 16:25:31 +0100
> 
> Eli, are the docs in patch #1 OK?

Yes, thanks.

Re: [PATCH v5 4/4] Make breakpoint/location number parsing error output consistent

[Xavier Roirand]

Pedro,

Thanks for all of your work (code enhancement & test optimisation). I've 
tested it and it works fine :)

Notice the typo in gdb/breakpoint.c at line 14319:

> x.y   where 'x' is a breakpoint number and 'z' a location number

should be:

> x.y   where 'x' is a breakpoint number and 'y' a location number

Please could you commit your changes ?
Regards

Le 10/26/17 à 5:25 PM, Pedro Alves a écrit :
> ... and also make GDB catch a few more cases of invalid input.
> 
> This fixes the inconsistency in GDB's output (e.g., "bad" vs "Bad")
> exposed by the new tests added in the previous commit.
> 
> Also, makes the "0-0" and "inverted range" cases be loud errors.
> 
> Also makes GDB reject negative breakpoint number in ranges.  We
> already rejected negative number literals, but you could still subvert
> that via convenience variables, like:
> 
>    (gdb) set $bp -1
>    (gdb) disable $bp.1-2
> 
> The change to get_number_trailer fixes a bug exposed by the new tests.
> The function did not handle parsing "-$num".  [This wasn't visible in
> the gdb.multi/tids.exp (which has similar tests) because the TID range
> parsing is implemented differently.]
> 
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	* breakpoint.c (extract_bp_kind): New enum.
> 	(extract_bp_num, extract_bp_or_bp_range): New functions, partially
> 	factored out from ...
> 	(extract_bp_number_and_location): ... here.
> 	* cli/cli-utils.c (get_number_trailer): Handle '-$variable'.
> 
> gdb/testsuite/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.base/ena-dis-br.exp (test_ena_dis_br): Adjust test.
> 	* gdb.cp/ena-dis-br-range.exp: Adjust tests.
> 	(disable_invalid, disable_inverted, disable_negative): New
> 	procedures.
> 	("bad numbers"): New set of tests.
> ---
>   gdb/breakpoint.c                          | 141 +++++++++++++++++++-----------
>   gdb/cli/cli-utils.c                       |  16 ++--
>   gdb/testsuite/gdb.base/ena-dis-br.exp     |   2 +-
>   gdb/testsuite/gdb.cp/ena-dis-br-range.exp | 115 +++++++++++++++++++-----
>   4 files changed, 191 insertions(+), 83 deletions(-)
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 43a7d87..ebf28de 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -14242,6 +14242,87 @@ find_location_by_number (int bp_num, int loc_num)
>     error (_("Bad breakpoint location number '%d'"), loc_num);
>   }
>   
> +/* Modes of operation for extract_bp_num.  */
> +enum class extract_bp_kind
> +{
> +  /* Extracting a breakpoint number.  */
> +  bp,
> +
> +  /* Extracting a location number.  */
> +  loc,
> +};
> +
> +/* Extract a breakpoint or location number (as determined by KIND)
> +   from the string starting at START.  TRAILER is a character which
> +   can be found after the number.  If you don't want a trailer, use
> +   '\0'.  If END_OUT is not NULL, it is set to point after the parsed
> +   string.  This always returns a positive integer.  */
> +
> +static int
> +extract_bp_num (extract_bp_kind kind, const char *start,
> +		int trailer, const char **end_out = NULL)
> +{
> +  const char *end = start;
> +  int num = get_number_trailer (&end, trailer);
> +  if (num < 0)
> +    error (kind == extract_bp_kind::bp
> +	   ? _("Negative breakpoint number '%.*s'")
> +	   : _("Negative breakpoint location number '%.*s'"),
> +	   int (end - start), start);
> +  if (num == 0)
> +    error (kind == extract_bp_kind::bp
> +	   ? _("Bad breakpoint number '%.*s'")
> +	   : _("Bad breakpoint location number '%.*s'"),
> +	   int (end - start), start);
> +
> +  if (end_out != NULL)
> +    *end_out = end;
> +  return num;
> +}
> +
> +/* Extract a breakpoint or location range (as determined by KIND) in
> +   the form NUM1-NUM2 stored at &ARG[arg_offset].  Returns a std::pair
> +   representing the (inclusive) range.  The returned pair's elements
> +   are always positive integers.  */
> +
> +static std::pair<int, int>
> +extract_bp_or_bp_range (extract_bp_kind kind,
> +			const std::string &arg,
> +			std::string::size_type arg_offset)
> +{
> +  std::pair<int, int> range;
> +  const char *bp_loc = &arg[arg_offset];
> +  std::string::size_type dash = arg.find ('-', arg_offset);
> +  if (dash != std::string::npos)
> +    {
> +      /* bp_loc is a range (x-z).  */
> +      if (arg.length () == dash + 1)
> +	error (kind == extract_bp_kind::bp
> +	       ? _("Bad breakpoint number at or near: '%s'")
> +	       : _("Bad breakpoint location number at or near: '%s'"),
> +	       bp_loc);
> +
> +      const char *end;
> +      const char *start_first = bp_loc;
> +      const char *start_second = &arg[dash + 1];
> +      range.first = extract_bp_num (kind, start_first, '-');
> +      range.second = extract_bp_num (kind, start_second, '\0', &end);
> +
> +      if (range.first > range.second)
> +	error (kind == extract_bp_kind::bp
> +	       ? _("Inverted breakpoint range at '%.*s'")
> +	       : _("Inverted breakpoint location range at '%.*s'"),
> +	       int (end - start_first), start_first);
> +    }
> +  else
> +    {
> +      /* bp_loc is a single value.  */
> +      range.first = extract_bp_num (kind, bp_loc, '\0');
> +      range.second = range.first;
> +    }
> +  return range;
> +}
> +
>   /* Extract the breakpoint/location range specified by ARG.  Returns
>      the breakpoint range in BP_NUM_RANGE, and the location range in
>      BP_LOC_RANGE.
> @@ -14267,69 +14348,23 @@ extract_bp_number_and_location (const std::string &arg,
>         /* Handle 'x.y' and 'x.y-z' cases.  */
>   
>         if (arg.length () == dot + 1 || dot == 0)
> -	error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
> -
> -      const char *ptb = arg.c_str ();
> -      int bp_num = get_number_trailer (&ptb, '.');
> -      if (bp_num == 0)
> -	error (_("Bad breakpoint number '%s'"), arg.c_str ());
> +	error (_("Bad breakpoint number at or near: '%s'"), arg.c_str ());
>   
> -      bp_num_range.first = bp_num;
> -      bp_num_range.second = bp_num;
> +      bp_num_range.first
> +	= extract_bp_num (extract_bp_kind::bp, arg.c_str (), '.');
> +      bp_num_range.second = bp_num_range.first;
>   
> -      const char *bp_loc = &arg[dot + 1];
> -      std::string::size_type dash = arg.find ('-', dot + 1);
> -      if (dash != std::string::npos)
> -	{
> -	  /* bp_loc is a range (x-z).  */
> -	  if (arg.length () == dash + 1)
> -	    error (_("bad breakpoint number at or near: '%s'"), bp_loc);
> -
> -	  const char *ptlf = bp_loc;
> -	  bp_loc_range.first = get_number_trailer (&ptlf, '-');
> -
> -	  const char *ptls = &arg[dash + 1];
> -	  bp_loc_range.second = get_number_trailer (&ptls, '\0');
> -	}
> -      else
> -	{
> -	  /* bp_loc is a single value.  */
> -	  const char *ptls = bp_loc;
> -	  bp_loc_range.first = get_number_trailer (&ptls, '\0');
> -	  if (bp_loc_range.first == 0)
> -	    error (_("bad breakpoint number at or near '%s'"), arg.c_str ());
> -	  bp_loc_range.second = bp_loc_range.first;
> -	}
> +      bp_loc_range = extract_bp_or_bp_range (extract_bp_kind::loc,
> +					     arg, dot + 1);
>       }
>     else
>       {
>         /* Handle x and x-y cases.  */
> -      std::string::size_type dash = arg.find ('-');
> -      if (dash != std::string::npos)
> -	{
> -	  if (arg.length () == dash + 1 || dash == 0)
> -	    error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
> -
> -	  const char *ptf = arg.c_str ();
> -	  bp_num_range.first = get_number_trailer (&ptf, '-');
>   
> -	  const char *pts = &arg[dash + 1];
> -	  bp_num_range.second = get_number_trailer (&pts, '\0');
> -	}
> -      else
> -	{
> -	  const char *ptf = arg.c_str ();
> -	  bp_num_range.first = get_number (&ptf);
> -	  if (bp_num_range.first == 0)
> -	    error (_("bad breakpoint number at or near '%s'"), arg.c_str ());
> -	  bp_num_range.second = bp_num_range.first;
> -	}
> +      bp_num_range = extract_bp_or_bp_range (extract_bp_kind::bp, arg, 0);
>         bp_loc_range.first = 0;
>         bp_loc_range.second = 0;
>       }
> -
> -  if (bp_num_range.first == 0 || bp_num_range.second == 0)
> -    error (_("bad breakpoint number at or near: '%s'"), arg.c_str ());
>   }
>   
>   /* Enable or disable a breakpoint location BP_NUM.LOC_NUM.  ENABLE
> diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
> index d5273b5..6b36378 100644
> --- a/gdb/cli/cli-utils.c
> +++ b/gdb/cli/cli-utils.c
> @@ -30,6 +30,13 @@ get_number_trailer (const char **pp, int trailer)
>   {
>     int retval = 0;	/* default */
>     const char *p = *pp;
> +  bool negative = false;
> +
> +  if (*p == '-')
> +    {
> +      ++p;
> +      negative = true;
> +    }
>   
>     if (*p == '$')
>       {
> @@ -70,11 +77,10 @@ get_number_trailer (const char **pp, int trailer)
>       }
>     else
>       {
> -      if (*p == '-')
> -	++p;
> +      const char *p1 = p;
>         while (*p >= '0' && *p <= '9')
>   	++p;
> -      if (p == *pp)
> +      if (p == p1)
>   	/* There is no number here.  (e.g. "cond a == b").  */
>   	{
>   	  /* Skip non-numeric token.  */
> @@ -84,7 +90,7 @@ get_number_trailer (const char **pp, int trailer)
>   	  retval = 0;
>   	}
>         else
> -	retval = atoi (*pp);
> +	retval = atoi (p1);
>       }
>     if (!(isspace (*p) || *p == '\0' || *p == trailer))
>       {
> @@ -95,7 +101,7 @@ get_number_trailer (const char **pp, int trailer)
>       }
>     p = skip_spaces (p);
>     *pp = p;
> -  return retval;
> +  return negative ? -retval : retval;
>   }
>   
>   /* See documentation in cli-utils.h.  */
> diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
> index 2bbb734..8655421 100644
> --- a/gdb/testsuite/gdb.base/ena-dis-br.exp
> +++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
> @@ -398,7 +398,7 @@ proc test_ena_dis_br { what } {
>       # Now enable(disable) '$b4.1 fooobaar'.  This should error on
>       # fooobaar.
>       gdb_test "$what $b4.1 fooobaar" \
> -       "bad breakpoint number at or near 'fooobaar'" \
> +       "Bad breakpoint number 'fooobaar'" \
>          "$what \$b4.1 fooobar"
>       set test1 "${what}d \$b4.1"
>   
> diff --git a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
> index 0ebc45f..629b0bc 100644
> --- a/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
> +++ b/gdb/testsuite/gdb.cp/ena-dis-br-range.exp
> @@ -111,9 +111,9 @@ gdb_test "enable 2.3-5" "Bad breakpoint location number '5'" \
>   gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
>       "breakpoint info enable 2.3 to 2.5"
>   
> -# Check that disabling an reverse location breakpoint range does not
> +# Check that disabling an inverted breakpoint location range does not
>   # work.
> -gdb_test_no_output "disable 2.3-2"
> +gdb_test "disable 2.3-2" "Inverted breakpoint location range at '3-2'"
>   
>   gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
>       "breakpoint info disable 2.3-2"
> @@ -128,44 +128,111 @@ gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
>   
>   # Check that disabling an invalid breakpoint location range does not
>   # cause trouble.
> -gdb_test_no_output "disable 2.8-6"
> +gdb_test "disable 2.8-6" "Inverted breakpoint location range at '8-6'"
>   
>   gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
>       "breakpoint info disable 2.8-6"
>   
>   # Check that invalid/open ranges are handled correctly.
>   with_test_prefix "open range" {
> -    gdb_test "disable -" "bad breakpoint number at or near: '-'"
> -    gdb_test "disable -1" "bad breakpoint number at or near: '-1'"
> -    gdb_test "disable 1-" "bad breakpoint number at or near: '1-'"
> -    gdb_test "disable 1.-2" "Bad breakpoint location number '-2'"
> -    gdb_test "disable 1.2-" "bad breakpoint number at or near: '2-'"
> -    gdb_test "disable 1.-2-3" "Bad breakpoint location number '-2'"
> -    gdb_test "disable 1-2-3" "bad breakpoint number at or near: '1-2-3'"
> +    gdb_test "disable -" "Bad breakpoint number at or near: '-'"
> +    gdb_test "disable -1" "Negative breakpoint number '-1'"
> +    gdb_test "disable 1-" "Bad breakpoint number at or near: '1-'"
> +    gdb_test "disable 1.-2" "Negative breakpoint location number '-2'"
> +    gdb_test "disable 1.2-" "Bad breakpoint location number at or near: '2-'"
> +    gdb_test "disable 1.-2-3" "Negative breakpoint location number '-2'"
> +    gdb_test "disable 1-2-3" "Bad breakpoint number '2-3'"
>   }
>   
>   with_test_prefix "dangling period" {
> -    gdb_test "disable 2." "bad breakpoint number at or near: '2.'"
> -    gdb_test "disable .2" "bad breakpoint number at or near: '.2'"
> -    gdb_test "disable 2.3.4" "bad breakpoint number at or near '2.3.4'"
> +    gdb_test "disable 2." "Bad breakpoint number at or near: '2.'"
> +    gdb_test "disable .2" "Bad breakpoint number at or near: '.2'"
> +    gdb_test "disable 2.3.4" "Bad breakpoint location number '3.4'"
>   }
>   
>   # Check that 0s are handled correctly.
>   with_test_prefix "zero" {
> -    gdb_test "disable 0" "bad breakpoint number at or near '0'"
> -    gdb_test "disable 0.0" "Bad breakpoint number '0.0'"
> -    gdb_test "disable 0.1" "Bad breakpoint number '0.1'"
> -    gdb_test "disable 0.1-2" "Bad breakpoint number '0.1-2'"
> -    gdb_test "disable 2.0" "bad breakpoint number at or near '2.0'"
> +    gdb_test "disable 0" "Bad breakpoint number '0'"
> +    gdb_test "disable 0.0" "Bad breakpoint number '0'"
> +    gdb_test "disable 0.1" "Bad breakpoint number '0'"
> +    gdb_test "disable 0.1-2" "Bad breakpoint number '0'"
> +    gdb_test "disable 2.0" "Bad breakpoint location number '0'"
> +    gdb_test "disable 2.0-0" "Bad breakpoint location number '0'"
> +    gdb_test "disable 2.0-1" "Bad breakpoint location number '0'"
> +    gdb_test "disable 2.1-0" "Bad breakpoint location number '0'"
> +}
> +
> +# Test "disable BPLIST" with an invalid breakpoint/location BPLIST.
> +# PREFIX and SUFFIX are concatenated to form BPLIST.  The invalid part
> +# is always in SUFFIX.
> +
> +proc disable_invalid {prefix suffix} {
> +    set bad_re [string_to_regexp $suffix]
> +
> +    if {$prefix == ""} {
> +	gdb_test \
> +	    "disable $suffix" \
> +	    "Bad breakpoint number '${bad_re}'"
> +    } else {
> +	gdb_test \
> +	    "disable ${prefix}$suffix" \
> +	    "Bad breakpoint location number '${bad_re}'"
> +    }
> +}
>   
> -    # These should really fail...
> -    gdb_test_no_output "disable 2.0-0"
> -    gdb_test_no_output "enable 2.0-0"
> +# Like disable_invalid, but expects an "inverted range" error.
>   
> -    gdb_test "disable 2.0-1" "Bad breakpoint location number '0'"
> +proc disable_inverted {prefix suffix} {
> +    set bad_re [string_to_regexp $suffix]
> +
> +    if {$prefix == ""} {
> +	gdb_test \
> +	    "disable $suffix" \
> +	    "Inverted breakpoint range at '${bad_re}'"
> +    } else {
> +	gdb_test \
> +	    "disable ${prefix}$suffix" \
> +	    "Inverted breakpoint location range at '${bad_re}'"
> +    }
> +}
> +
> +# Like disable_invalid, but expects a "negative number" error.
> +
> +proc disable_negative {prefix suffix} {
> +    set bad_re [string_to_regexp $suffix]
> +
> +    if {$prefix == ""} {
> +	gdb_test \
> +	    "disable $suffix" \
> +	    "Negative breakpoint number '${bad_re}'"
> +    } else {
> +	gdb_test \
> +	    "disable ${prefix}$suffix" \
> +	    "Negative breakpoint location number '${bad_re}'"
> +    }
> +}
>   
> -    # Likewise, should fail.
> -    gdb_test_no_output "disable 2.1-0"
> +with_test_prefix "bad numbers" {
> +    gdb_test "p \$zero = 0" " = 0"
> +    gdb_test "p \$one = 1" " = 1"
> +    gdb_test "p \$two = 2" " = 2"
> +    gdb_test "p \$minus_one = -1" " = -1"
> +    foreach prefix {"" "1." "$one."} {
> +	set prefix_re [string_to_regexp $prefix]
> +
> +	disable_invalid $prefix "foo"
> +	disable_invalid $prefix "1foo"
> +	disable_invalid $prefix "foo1"
> +
> +	disable_inverted $prefix "2-1"
> +	disable_inverted $prefix "2-\$one"
> +	disable_inverted $prefix "\$two-1"
> +	disable_inverted $prefix "\$two-\$one"
> +
> +	disable_negative $prefix "-1"
> +	disable_negative $prefix "-\$one"
> +	disable_negative $prefix "\$minus_one"
> +    }
>   }
>   
>   gdb_test "info break" [make_info_breakpoint_reply_re y y y y y y] \
> 

Re: [PATCH v5 4/4] Make breakpoint/location number parsing error output consistent

[Pedro Alves]

Hi Xavier,

On 11/03/2017 08:54 AM, Xavier Roirand wrote:
> Pedro,
> 
> Thanks for all of your work (code enhancement & test optimisation). I've
> tested it and it works fine :)
> 
> Notice the typo in gdb/breakpoint.c at line 14319:
> 
>> x.y   where 'x' is a breakpoint number and 'z' a location number
> 
> should be:
> 
>> x.y   where 'x' is a breakpoint number and 'y' a location number

Thanks, fixed and ...

> 
> Please could you commit your changes ?

... now pushed.

Thanks,
Pedro Alves