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(-)
@@ -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);
@@ -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)
{
@@ -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
@@ -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)
@@ -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" \