[PATCH 4/5] get_number returns status in details

[Yao Qi <yao at codesourcery dot com>]

Hi,
get_number doesn't return status in details, which prevent callers
printing error or warning in details.  This patch change get_number
interface to return status in return value and return parsed number in
a new argument '*number'.  It is quite similar to to_xfer_partial
interface.

After this patch, from the return value of get_number, we can tell the
input is invalid or the input is zero.  Then, we can add the checking
to "enable count" command, in which, zero is a valid input.

GDB is quiet for these invalid input like this

  (gdb) enable count 1.1 1
  (gdb)

with this patch, GDB emits error if input number isn't valid:

  (gdb) enable count 1.1 1
  Invalid count number '1.1 1'

zero is still a valid number, so I add a test about
"enable count 0 1" and this input is valid.  However, the semantics of
setting count to zero is out of the scope of this patch.

gdb:

2014-03-13  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (enable_count_command): Error out if
	get_number returns error.
	* cli/cli-utils.c (get_number_trailer): Update comments.
	(get_number_trailer): Add argument number and change return
	type to enum get_number_status.  Return status instead of
	printing messages.
	(print_get_number_status): New function.
	(get_number): Add argument number and call
	print_get_number_status.  All callers updated.
	(get_number_or_range): Call print_get_number_status.
	* cli/cli-utils.h (enum get_number_status): New.
	(get_number): Update declaration.
	* reverse.c (goto_bookmark_command): Change 'num' to type int.

gdb/testsuite:

2014-03-13  Yao Qi  <yao@codesourcery.com>

	* gdb.base/ena-dis-br.exp: Test invalid count number and zero.
---
 gdb/breakpoint.c                      |   22 ++++-----
 gdb/cli/cli-utils.c                   |   84 +++++++++++++++++++++-----------
 gdb/cli/cli-utils.h                   |   29 ++++++++++-
 gdb/reverse.c                         |    6 +--
 gdb/testsuite/gdb.base/ena-dis-br.exp |    9 ++++
 5 files changed, 102 insertions(+), 48 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 211c2aa..6ccc0e7 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1040,8 +1040,7 @@ condition_command (char *arg, int from_tty)
     error_no_arg (_("breakpoint number"));
 
   p = arg;
-  bnum = get_number (&p);
-  if (bnum == 0)
+  if (get_number (&p, &bnum) != GET_NUMBER_OK)
     error (_("Bad breakpoint argument: '%s'"), arg);
 
   ALL_BREAKPOINTS (b)
@@ -14564,8 +14563,7 @@ ignore_command (char *args, int from_tty)
   if (p == 0)
     error_no_arg (_("a breakpoint number"));
 
-  num = get_number (&p);
-  if (num == 0)
+  if (get_number (&p, &num) != GET_NUMBER_OK)
     error (_("bad breakpoint number: '%s'"), args);
   if (*p == 0)
     error (_("Second argument (specified ignore-count) is missing."));
@@ -14634,8 +14632,7 @@ find_location_by_number (char *number)
   *dot = '\0';
 
   p1 = number;
-  bp_num = get_number (&p1);
-  if (bp_num == 0)
+  if (get_number (&p1, &bp_num) != GET_NUMBER_OK)
     error (_("Bad breakpoint number '%s'"), number);
 
   ALL_BREAKPOINTS (b)
@@ -14648,8 +14645,7 @@ find_location_by_number (char *number)
     error (_("Bad breakpoint number '%s'"), number);
   
   p1 = dot+1;
-  loc_num = get_number (&p1);
-  if (loc_num == 0)
+  if (get_number (&p1, &loc_num) != GET_NUMBER_OK)
     error (_("Bad breakpoint location number '%s'"), number);
 
   --loc_num;
@@ -14934,7 +14930,11 @@ do_map_enable_count_breakpoint (struct breakpoint *bpt, void *countptr)
 static void
 enable_count_command (char *args, int from_tty)
 {
-  int count = get_number (&args);
+  char *p = args;
+  int count;
+
+  if (get_number (&args, &count) != GET_NUMBER_OK)
+    error (_("Invalid count number '%s'"), p);
 
   map_breakpoint_numbers (args, do_map_enable_count_breakpoint, &count);
 }
@@ -15611,9 +15611,7 @@ get_tracepoint_by_number (char *arg)
     {
       char *instring = arg;
 
-      tpnum = get_number (&arg);
-
-      if (tpnum == 0)
+      if (get_number (&arg, &tpnum) != GET_NUMBER_OK)
 	{
 	  printf_filtered (_("bad tracepoint number at or near '%s'\n"),
 			   instring);
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index d59a231..53211a4 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -25,19 +25,15 @@
 
 #include <ctype.h>
 
-/* *PP is a string denoting a number.  Get the number of the.  Advance
-   *PP after the string and any trailing whitespace.
+/* TRAILER is a character which can be found after the number; most
+   commonly this is `-'.  If you don't want a trailer, use \0.  For
+   PP and function return value, please see documentation of
+   get_number in cli-utils.h.  */
 
-   Currently the string can either be a number, or "$" followed by the
-   name of a convenience variable, or ("$" or "$$") followed by digits.
-
-   TRAILER is a character which can be found after the number; most
-   commonly this is `-'.  If you don't want a trailer, use \0.  */
-
-static int
-get_number_trailer (char **pp, int trailer)
+static enum get_number_status
+get_number_trailer (char **pp, int trailer, int *number)
 {
-  int retval = 0;	/* default */
+  enum get_number_status retval;	/* default */
   char *p = *pp;
 
   if (*p == '$')
@@ -47,12 +43,13 @@ get_number_trailer (char **pp, int trailer)
       if (val)	/* Value history reference */
 	{
 	  if (TYPE_CODE (value_type (val)) == TYPE_CODE_INT)
-	    retval = value_as_long (val);
-	  else
 	    {
-	      printf_filtered (_("History value must have integer type.\n"));
-	      retval = 0;
+	      retval = GET_NUMBER_OK;
+	      *number = (int) value_as_long (val);
 	    }
+	  else
+	    retval = GET_NUMBER_E_HISTORY_VALUE_TYPE;
+
 	}
       else	/* Convenience variable */
 	{
@@ -68,13 +65,12 @@ get_number_trailer (char **pp, int trailer)
 	  strncpy (varname, start, p - start);
 	  varname[p - start] = '\0';
 	  if (get_internalvar_integer (lookup_internalvar (varname), &val))
-	    retval = (int) val;
-	  else
 	    {
-	      printf_filtered (_("Convenience variable must "
-				 "have integer value.\n"));
-	      retval = 0;
+	      *number = (int) val;
+	      retval = GET_NUMBER_OK;
 	    }
+	  else
+	    retval = GET_NUMBER_E_CONVENIENCE_TYPE;
 	}
     }
   else
@@ -90,29 +86,54 @@ get_number_trailer (char **pp, int trailer)
 	  while (*p && !isspace((int) *p))
 	    ++p;
 	  /* Return zero, which caller must interpret as error.  */
-	  retval = 0;
+	  retval = GET_NUMBER_E_NOT_NUMBER;
 	}
       else
-	retval = atoi (*pp);
+	{
+	  *number = atoi (*pp);
+	  retval = GET_NUMBER_OK;
+	}
     }
   if (!(isspace (*p) || *p == '\0' || *p == trailer))
     {
       /* Trailing junk: return 0 and let caller print error msg.  */
       while (!(isspace (*p) || *p == '\0' || *p == trailer))
 	++p;
-      retval = 0;
+
+      retval = GET_NUMBER_E_TRAILING_JUNK;
     }
   p = skip_spaces (p);
   *pp = p;
   return retval;
 }
 
+/* Print STATUS.  */
+
+static void
+print_get_number_status (enum get_number_status status)
+{
+  if (status == GET_NUMBER_E_HISTORY_VALUE_TYPE)
+    printf_filtered (_("History value must have integer type.\n"));
+  else if (status == GET_NUMBER_E_CONVENIENCE_TYPE)
+    printf_filtered (_("Convenience variable must have integer value.\n"));
+  else if (status == GET_NUMBER_E_NOT_NUMBER)
+    printf_filtered (_("Not a number.\n"));
+  else if (status == GET_NUMBER_E_TRAILING_JUNK)
+    printf_filtered (_("Junk at the end of number.\n"));
+}
+
 /* See documentation in cli-utils.h.  */
 
-int
-get_number (char **pp)
+enum get_number_status
+get_number (char **pp, int *number)
 {
-  return get_number_trailer (pp, '\0');
+  enum get_number_status status;
+
+  status = get_number_trailer (pp, '\0', number);
+
+  print_get_number_status (status);
+
+  return status;
 }
 
 /* See documentation in cli-utils.h.  */
@@ -132,9 +153,15 @@ get_number_or_range (struct get_number_or_range_state *state)
 {
   if (*state->string != '-')
     {
+      enum get_number_status status;
+
+      status = get_number_trailer (&state->string, '-', &state->last_retval);
       /* Default case: state->string is pointing either to a solo
 	 number, or to the first number of a range.  */
-      state->last_retval = get_number_trailer (&state->string, '-');
+      if (GET_NUMBER_OK != status)
+	state->last_retval = 0;
+
+      print_get_number_status (status);
       if (*state->string == '-')
 	{
 	  char **temp;
@@ -145,8 +172,7 @@ get_number_or_range (struct get_number_or_range_state *state)
 
 	  temp = &state->end_ptr; 
 	  state->end_ptr = skip_spaces (state->string + 1);
-	  state->end_value = get_number (temp);
-	  if (state->end_value == 0)
+	  if (get_number (temp, &state->end_value) != GET_NUMBER_OK)
 	    error (_("wrong value"));
 	  else if (state->end_value < state->last_retval) 
 	    {
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index fed876b..eee23f4 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -20,13 +20,36 @@
 #ifndef CLI_UTILS_H
 #define CLI_UTILS_H
 
-/* *PP is a string denoting a number.  Get the number of the.  Advance
-   *PP after the string and any trailing whitespace.
+/* The returned status of get_number.  */
+
+enum get_number_status
+{
+  /* The input string is correctly parsed.  */
+  GET_NUMBER_OK = 0,
+
+  /* The input string is a history value, but its type isn't
+     expected.  */
+  GET_NUMBER_E_HISTORY_VALUE_TYPE = -1,
+
+  /* The input string is a convenience variable, but its type
+     isn't expected.  */
+  GET_NUMBER_E_CONVENIENCE_TYPE = -2,
+
+  /* The input string is not a number.  */
+  GET_NUMBER_E_NOT_NUMBER = -3,
+
+  /* There is junk at the end of the input string.  */
+  GET_NUMBER_E_TRAILING_JUNK = -4,
+};
+
+/* *PP is a string denoting a number.  Return the status of
+   'enum get_number_status' and save the number in *NUMBER on
+   success.  Advance *PP after the string and any trailing whitespace.
 
    Currently the string can either be a number,  or "$" followed by the
    name of a convenience variable, or ("$" or "$$") followed by digits.  */
 
-extern int get_number (char **);
+extern enum get_number_status get_number (char **pp, int *number);
 
 /* An object of this type is passed to get_number_or_range.  It must
    be initialized by calling init_number_or_range.  This type is
diff --git a/gdb/reverse.c b/gdb/reverse.c
index 30a0328..7828555 100644
--- a/gdb/reverse.c
+++ b/gdb/reverse.c
@@ -249,7 +249,7 @@ static void
 goto_bookmark_command (char *args, int from_tty)
 {
   struct bookmark *b;
-  unsigned long num;
+  int num;
   char *p = args;
 
   if (args == NULL || args[0] == '\0')
@@ -274,9 +274,7 @@ goto_bookmark_command (char *args, int from_tty)
     }
 
   /* General case.  Bookmark identified by bookmark number.  */
-  num = get_number (&args);
-
-  if (num == 0)
+  if (get_number (&args, &num) != GET_NUMBER_OK)
     error (_("goto-bookmark: invalid bookmark number '%s'."), p);
 
   ALL_BOOKMARKS (b)
diff --git a/gdb/testsuite/gdb.base/ena-dis-br.exp b/gdb/testsuite/gdb.base/ena-dis-br.exp
index 6f2c469..7d0e3e1 100644
--- a/gdb/testsuite/gdb.base/ena-dis-br.exp
+++ b/gdb/testsuite/gdb.base/ena-dis-br.exp
@@ -155,6 +155,15 @@ set bp [break_at $bp_location7 "line $bp_location7"]
 
 set bp2 [break_at marker1 " line ($bp_location15|$bp_location16)"]
 
+# Test enable count with invalid count number.
+
+gdb_test "enable count 1.1 $bp" "Invalid count number '1.1 $bp'.*" \
+    "enable count with invalid number"
+
+# Test enable count with zero, which is valid.
+
+gdb_test_no_output "enable count 0 $bp" "enable count with zero"
+
 gdb_test_no_output "enable count 2 $bp" "disable break with count"
 
 gdb_test "continue" \
-- 
1.7.7.6