Re: [RFA 19/22] Convert tid_range_parser to class

Pedro Alves
2016-10-04T23:09:29Z

On 10/04/2016 08:24 PM, Pedro Alves wrote:

> Anyway, I have to disappear now. Will continue later.

Here's what I ended up with, after reading your version
in more detail. That brought in the "deleted" copy constructor/assignment
operators [1], and "const" in several methods, which I then propagated
to number_or_range_parser too.

WDYT?

[1] - I'm pondering making those use =delete if compiling with
a C++ compiler, behind some

#define DISABLE_COPY_AND_ASSIGN(TYPE) ...

macro.

Or, maybe add a gdb::noncopyable class that non-copyable
classes inherit from privately, just like boost:noncopyable.
Anyone got experience on the latter, and whether that turns
out to be a bad idea?

Anyway, here's the current version of the patch. Tested on
x86_64 Fedora 23.

Let me know what you think.

From 4b57514dce9358f7e4367b9dcd870253e3ab249b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 4 Oct 2016 23:28:31 +0100
Subject: [PATCH] Convert tid_range_parser and get_number_or_range to classes

This converts tid_range_parser and get_number_or_range to be classes.
The various tid_range_parser_* and get_number_or_range_* functions
become methods on the respective classes. Then it updates the users
to follow.

The rationale for the change is that this provides better
encapsulation. For example, this forced me to think of a better
interface between tid_range_parser and get_number_or_range, since the
former peeked into the latter's internals a bit too much. That ended
up resulting mostly in these two not-just-straight-1-1 changes:

void
-tid_range_parser_skip (struct tid_range_parser *parser)
+tid_range_parser::skip_range ()
{
...

- tid_range_parser_init (parser, parser->range_parser.end_ptr,
- parser->default_inferior);
+ m_range_parser.skip_range ();
+ init (m_range_parser.string (), m_default_inferior);
}

and:

/* If we successfully parsed a thread number or finished parsing a
thread range, switch back to assuming the next TID is
inferior-qualified. */
- if (parser->range_parser.end_ptr == NULL
- || parser->range_parser.string == parser->range_parser.end_ptr)
+ if (!m_range_parser.in_range ())
{

For the same reason (encapsulation), this moves the enum
tid_range_state definition to within the tid_parser class's scope,
since that is private implementation detail.

While at it, switch to use "bool" for booleans.

gdb/ChangeLog:
2016-10-04 Pedro Alves <palves@redhat.com>
Tom Tromey <tom@tromey.com>

* tid-parse.h (tid_range_parser): New class.
(enum tid_range_state): Move into tid_range_parser's scope.
Remove TID_RANGE_ prefix from all values.
(tid_range_parser_get_tid, tid_range_parser_get_tid_range)
(tid_range_parser_star_range, tid_range_parser_finished)
(tid_range_parser_skip, tid_range_parser_qualified): Don't
declare.
(tid_is_in_list): Update comment.
* tid-parse.c (tid_range_parser::tid_range_parser): New.
(init, finished, get_string, skip, tid_is_qualified)
(get_tid_or_range, get_tid_range, get_tid, star_range): Rename;
turn into methods.
(tid_is_in_list): Adjust.
* cli/cli-utils.h (number_or_range_parser): New class.
(init_number_or_range, get_number_or_range)
(number_range_setup_range): Don't declare.
* cli/cli-utils.c
(number_or_range_parser::number_or_range_parser): New.
(init_number_or_range, get_number_or_range)
(number_range_setup_range): Rename; turn into methods.
(number_is_in_list): Adjust.
* breakpoint.c (map_breakpoint_numbers): Adjust. Use bool.
(trace_pass_command, get_tracepoint_by_number): Adjust.
* breakpoint.h (get_tracepoint_by_number): Adjust.
* inferior.c (detach_inferior_command, kill_inferior_command)
(remove_inferior_command): Adjust.
* linespec.c (decode_line_2): Adjust.
* memattr.c (mem_enable_command, mem_disable_command)
(mem_delete_command): Adjust.
* printcmd.c (map_display_numbers): Adjust.
* reverse.c (delete_bookmark_command, bookmarks_info): Adjust.
* thread.c (thread_apply_command): Adjust.
---
gdb/breakpoint.c | 33 ++++-----
gdb/breakpoint.h | 6 +-
gdb/cli/cli-utils.c | 76 +++++++++++---------
gdb/cli/cli-utils.h | 114 ++++++++++++++++++------------
gdb/inferior.c | 32 ++++-----
gdb/linespec.c | 9 +--
gdb/memattr.c | 32 ++++-----
gdb/printcmd.c | 9 ++-
gdb/reverse.c | 19 ++---
gdb/thread.c | 27 ++++---
gdb/tid-parse.c | 170 ++++++++++++++++++++++-----------------------
gdb/tid-parse.h | 197 +++++++++++++++++++++++++++-------------------------
12 files changed, 365 insertions(+), 359 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1e05932..32d6a95 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14775,21 +14775,18 @@ map_breakpoint_numbers (char *args, void (*function) (struct breakpoint *,
{
int num;
struct breakpoint *b, *tmp;
- int match;
- struct get_number_or_range_state state;

if (args == 0 || *args == '\0')
error_no_arg (_("one or more breakpoint numbers"));

- init_number_or_range (&state, args);
+ number_or_range_parser parser (args);

- while (!state.finished)
+ while (!parser.finished ())
{
- const char *p = state.string;
+ const char *p = parser.cur_tok ();
+ bool match = false;

- match = 0;
-
- num = get_number_or_range (&state);
+ num = parser.get_number ();
if (num == 0)
{
warning (_("bad breakpoint number at or near '%s'"), p);
@@ -14799,11 +14796,11 @@ map_breakpoint_numbers (char *args, void (*function) (struct breakpoint *,
ALL_BREAKPOINTS_SAFE (b, tmp)
if (b->number == num)
{
- match = 1;
+ match = true;
function (b, data);
break;
}
- if (match == 0)
+ if (!match)
printf_unfiltered (_("No breakpoint number %d.\n"), num);
}
}
@@ -15584,12 +15581,10 @@ trace_pass_command (char *args, int from_tty)
}
else
{
- struct get_number_or_range_state state;
-
- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- t1 = get_tracepoint_by_number (&args, &state);
+ t1 = get_tracepoint_by_number (&args, &parser);
if (t1)
trace_pass_set_count (t1, count, from_tty);
}
@@ -15635,16 +15630,16 @@ get_tracepoint_by_number_on_target (int num)

struct tracepoint *
get_tracepoint_by_number (char **arg,
- struct get_number_or_range_state *state)
+ number_or_range_parser *parser)
{
struct breakpoint *t;
int tpnum;
char *instring = arg == NULL ? NULL : *arg;

- if (state)
+ if (parser != NULL)
{
- gdb_assert (!state->finished);
- tpnum = get_number_or_range (state);
+ gdb_assert (!parser->finished ());
+ tpnum = parser->get_number ();
}
else if (arg == NULL || *arg == NULL || ! **arg)
tpnum = tracepoint_count;
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 4bdf0d5..aaff3d5 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -31,7 +31,7 @@ struct value;
struct block;
struct gdbpy_breakpoint_object;
struct gdbscm_breakpoint_object;
-struct get_number_or_range_state;
+struct number_or_range_parser;
struct thread_info;
struct bpstats;
struct bp_location;
@@ -1587,8 +1587,8 @@ extern struct tracepoint *get_tracepoint_by_number_on_target (int num);

/* Find a tracepoint by parsing a number in the supplied string. */
extern struct tracepoint *
- get_tracepoint_by_number (char **arg,
- struct get_number_or_range_state *state);
+ get_tracepoint_by_number (char **arg,
+ number_or_range_parser *parser);

/* Return a vector of all tracepoints currently defined. The vector
is newly allocated; the caller should free when done with it. */
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index 0946db0..0fb68f2 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -121,39 +121,49 @@ get_number (char **pp)

/* See documentation in cli-utils.h. */

+number_or_range_parser::number_or_range_parser (const char *string)
+{
+ init (string);
+}
+
+/* See documentation in cli-utils.h. */
+
void
-init_number_or_range (struct get_number_or_range_state *state,
- const char *string)
+number_or_range_parser::init (const char *string)
{
- memset (state, 0, sizeof (*state));
- state->string = string;
+ m_finished = false;
+ m_cur_tok = string;
+ m_last_retval = 0;
+ m_end_value = 0;
+ m_end_ptr = NULL;
+ m_in_range = false;
}

/* See documentation in cli-utils.h. */

int
-get_number_or_range (struct get_number_or_range_state *state)
+number_or_range_parser::get_number ()
{
- if (state->in_range)
+ if (m_in_range)
{
/* All number-parsing has already been done. Return the next
integer value (one greater than the saved previous value).
Do not advance the token pointer until the end of range is
reached. */

- if (++state->last_retval == state->end_value)
+ if (++m_last_retval == m_end_value)
{
/* End of range reached; advance token pointer. */
- state->string = state->end_ptr;
- state->in_range = 0;
+ m_cur_tok = m_end_ptr;
+ m_in_range = false;
}
}
- else if (*state->string != '-')
+ else if (*m_cur_tok != '-')
{
- /* Default case: state->string is pointing either to a solo
+ /* Default case: state->m_cur_tok is pointing either to a solo
number, or to the first number of a range. */
- state->last_retval = get_number_trailer (&state->string, '-');
- if (*state->string == '-')
+ m_last_retval = get_number_trailer (&m_cur_tok, '-');
+ if (*m_cur_tok == '-')
{
const char **temp;

@@ -161,42 +171,42 @@ get_number_or_range (struct get_number_or_range_state *state)
Skip the '-', parse and remember the second number,
and also remember the end of the final token. */

- temp = &state->end_ptr;
- state->end_ptr = skip_spaces_const (state->string + 1);
- state->end_value = get_number_const (temp);
- if (state->end_value < state->last_retval)
+ temp = &m_end_ptr;
+ m_end_ptr = skip_spaces_const (m_cur_tok + 1);
+ m_end_value = get_number_const (temp);
+ if (m_end_value < m_last_retval)
{
error (_("inverted range"));
}
- else if (state->end_value == state->last_retval)
+ else if (m_end_value == m_last_retval)
{
/* Degenerate range (number1 == number2). Advance the
token pointer so that the range will be treated as a
- single number. */
- state->string = state->end_ptr;
+ single number. */
+ m_cur_tok = m_end_ptr;
}
else
- state->in_range = 1;
+ m_in_range = true;
}
}
else
error (_("negative value"));
- state->finished = *state->string == '\0';
- return state->last_retval;
+ m_finished = *m_cur_tok == '\0';
+ return m_last_retval;
}

/* See documentation in cli-utils.h. */

void
-number_range_setup_range (struct get_number_or_range_state *state,
- int start_value, int end_value, const char *end_ptr)
+number_or_range_parser::setup_range (int start_value, int end_value,
+ const char *end_ptr)
{
gdb_assert (start_value > 0);

- state->in_range = 1;
- state->end_ptr = end_ptr;
- state->last_retval = start_value - 1;
- state->end_value = end_value;
+ m_in_range = true;
+ m_end_ptr = end_ptr;
+ m_last_retval = start_value - 1;
+ m_end_value = end_value;
}

/* Accept a number and a string-form list of numbers such as is
@@ -210,15 +220,13 @@ number_range_setup_range (struct get_number_or_range_state *state,
int
number_is_in_list (const char *list, int number)
{
- struct get_number_or_range_state state;
-
if (list == NULL || *list == '\0')
return 1;

- init_number_or_range (&state, list);
- while (!state.finished)
+ number_or_range_parser parser (list);
+ while (!parser.finished ())
{
- int gotnum = get_number_or_range (&state);
+ int gotnum = parser.get_number ();

if (gotnum == 0)
error (_("Args must be numbers or '$' variables."));
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index a31fff5..3188bb7 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -39,65 +39,91 @@ extern int get_number_const (const char **);

extern int get_number (char **);

-/* 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
- defined here so that it can be stack-allocated, but all members
- other than `finished' and `string' should be treated as opaque. */
+/* Parse a number or a range.
+ A number will be of the form handled by get_number.
+ A range will be of the form <number1> - <number2>, and
+ will represent all the integers between number1 and number2,
+ inclusive. */

-struct get_number_or_range_state
+class number_or_range_parser
{
- /* Non-zero if parsing has completed. */
- int finished;
+public:
+ /* Default construction. Must call init before calling
+ get_next. */
+ number_or_range_parser () {}
+
+ /* Calls init automatically. */
+ number_or_range_parser (const char *string);
+
+ /* STRING is the string to be parsed. */
+ void init (const char *string);
+
+ /* While processing a range, this fuction is called iteratively; At
+ each call it will return the next value in the range.
+
+ At the beginning of parsing a range, the char pointer
+ STATE->m_cur_tok will be advanced past <number1> and left
+ pointing at the '-' token. Subsequent calls will not advance the
+ pointer until the range is completed. The call that completes
+ the range will advance the pointer past <number2>. */
+ int get_number ();
+
+ /* Setup internal state such that get_next() returns numbers in the
+ START_VALUE to END_VALUE range. END_PTR is where the string is
+ advanced to when get_next() returns END_VALUE. */
+ void setup_range (int start_value, int end_value,
+ const char *end_ptr);
+
+ /* Returns true if parsing has completed. */
+ bool finished () const
+ { return m_finished; }
+
+ /* Return the string being parsed. When parsing has finished, this
+ points past the last parsed token. */
+ const char *cur_tok () const
+ { return m_cur_tok; }
+
+ /* True when parsing a range. */
+ bool in_range () const
+ { return m_in_range; }
+
+ /* When parsing a range, the final value in the range. */
+ int end_value () const
+ { return m_end_value; }
+
+ /* When parsing a range, skip past the final token in the range. */
+ void skip_range ()
+ {
+ gdb_assert (m_in_range);
+ m_cur_tok = m_end_ptr;
+ }
+
+private:
+ /* No need for these. They are intentionally not defined anywhere. */
+ number_or_range_parser (const number_or_range_parser &);
+ number_or_range_parser &operator= (const number_or_range_parser &);
+
+ /* True if parsing has completed. */
+ bool m_finished;

/* The string being parsed. When parsing has finished, this points
past the last parsed token. */
- const char *string;
+ const char *m_cur_tok;

/* Last value returned. */
- int last_retval;
+ int m_last_retval;

/* When parsing a range, the final value in the range. */
- int end_value;
+ int m_end_value;

/* When parsing a range, a pointer past the final token in the
range. */
- const char *end_ptr;
+ const char *m_end_ptr;

- /* Non-zero when parsing a range. */
- int in_range;
+ /* True when parsing a range. */
+ bool m_in_range;
};

-/* Initialize a get_number_or_range_state for use with
- get_number_or_range_state. STRING is the string to be parsed. */
-
-extern void init_number_or_range (struct get_number_or_range_state *state,
- const char *string);
-
-/* Parse a number or a range.
- A number will be of the form handled by get_number.
- A range will be of the form <number1> - <number2>, and
- will represent all the integers between number1 and number2,
- inclusive.
-
- While processing a range, this fuction is called iteratively;
- At each call it will return the next value in the range.
-
- At the beginning of parsing a range, the char pointer STATE->string will
- be advanced past <number1> and left pointing at the '-' token.
- Subsequent calls will not advance the pointer until the range
- is completed. The call that completes the range will advance
- the pointer past <number2>. */
-
-extern int get_number_or_range (struct get_number_or_range_state *state);
-
-/* Setups STATE such that get_number_or_range returns numbers in range
- START_VALUE to END_VALUE. When get_number_or_range returns
- END_VALUE, the STATE string is advanced to END_PTR. */
-
-extern void number_range_setup_range (struct get_number_or_range_state *state,
- int start_value, int end_value,
- const char *end_ptr);
-
/* Accept a number and a string-form list of numbers such as is
accepted by get_number_or_range. Return TRUE if the number is
in the list.
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 277b988..1602483 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -651,17 +651,15 @@ print_inferior (struct ui_out *uiout, char *requested_inferiors)
static void
detach_inferior_command (char *args, int from_tty)
{
- int num, pid;
struct thread_info *tp;
- struct get_number_or_range_state state;

if (!args || !*args)
error (_("Requires argument (inferior id(s) to detach)"));

- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- num = get_number_or_range (&state);
+ int num = parser.get_number ();

if (!valid_gdb_inferior_id (num))
{
@@ -669,7 +667,7 @@ detach_inferior_command (char *args, int from_tty)
continue;
}

- pid = gdb_inferior_id_to_pid (num);
+ int pid = gdb_inferior_id_to_pid (num);
if (pid == 0)
{
warning (_("Inferior ID %d is not running."), num);
@@ -692,17 +690,15 @@ detach_inferior_command (char *args, int from_tty)
static void
kill_inferior_command (char *args, int from_tty)
{
- int num, pid;
struct thread_info *tp;
- struct get_number_or_range_state state;

if (!args || !*args)
error (_("Requires argument (inferior id(s) to kill)"));

- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- num = get_number_or_range (&state);
+ int num = parser.get_number ();

if (!valid_gdb_inferior_id (num))
{
@@ -710,7 +706,7 @@ kill_inferior_command (char *args, int from_tty)
continue;
}

- pid = gdb_inferior_id_to_pid (num);
+ int pid = gdb_inferior_id_to_pid (num);
if (pid == 0)
{
warning (_("Inferior ID %d is not running."), num);
@@ -788,18 +784,14 @@ info_inferiors_command (char *args, int from_tty)
static void
remove_inferior_command (char *args, int from_tty)
{
- int num;
- struct inferior *inf;
- struct get_number_or_range_state state;
-
if (args == NULL || *args == '\0')
error (_("Requires an argument (inferior id(s) to remove)"));

- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- num = get_number_or_range (&state);
- inf = find_inferior_id (num);
+ int num = parser.get_number ();
+ struct inferior *inf = find_inferior_id (num);

if (inf == NULL)
{
diff --git a/gdb/linespec.c b/gdb/linespec.c
index ccedec8..5182b45 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1328,7 +1328,6 @@ decode_line_2 (struct linespec_state *self,
int i;
struct cleanup *old_chain;
VEC (const_char_ptr) *filters = NULL;
- struct get_number_or_range_state state;
struct decode_line_2_item *items;
int items_count;

@@ -1409,12 +1408,10 @@ decode_line_2 (struct linespec_state *self,
if (args == 0 || *args == 0)
error_no_arg (_("one or more choice numbers"));

- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- int num;
-
- num = get_number_or_range (&state);
+ int num = parser.get_number ();

if (num == 0)
error (_("canceled"));
diff --git a/gdb/memattr.c b/gdb/memattr.c
index c4a3b75..1c5c48f 100644
--- a/gdb/memattr.c
+++ b/gdb/memattr.c
@@ -578,12 +578,10 @@ mem_enable_command (char *args, int from_tty)
}
else
{
- struct get_number_or_range_state state;
-
- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- num = get_number_or_range (&state);
+ num = parser.get_number ();
mem_enable (num);
}
}
@@ -610,27 +608,24 @@ mem_disable (int num)
static void
mem_disable_command (char *args, int from_tty)
{
- int num;
- struct mem_region *m;
- int ix;
-
require_user_regions (from_tty);

target_dcache_invalidate ();

if (args == NULL || *args == '\0')
{
+ struct mem_region *m;
+ int ix;
+
for (ix = 0; VEC_iterate (mem_region_s, mem_region_list, ix, m); ix++)
m->enabled_p = 0;
}
else
{
- struct get_number_or_range_state state;
-
- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- num = get_number_or_range (&state);
+ int num = parser.get_number ();
mem_disable (num);
}
}
@@ -666,9 +661,6 @@ mem_delete (int num)
static void
mem_delete_command (char *args, int from_tty)
{
- int num;
- struct get_number_or_range_state state;
-
require_user_regions (from_tty);

target_dcache_invalidate ();
@@ -681,10 +673,10 @@ mem_delete_command (char *args, int from_tty)
return;
}

- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- num = get_number_or_range (&state);
+ int num = parser.get_number ();
mem_delete (num);
}

diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index d4a4b9e..c7f477b 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1876,19 +1876,18 @@ map_display_numbers (char *args,
void *),
void *data)
{
- struct get_number_or_range_state state;
int num;

if (args == NULL)
error_no_arg (_("one or more display numbers"));

- init_number_or_range (&state, args);
+ number_or_range_parser parser (args);

- while (!state.finished)
+ while (!parser.finished ())
{
- const char *p = state.string;
+ const char *p = parser.cur_tok ();

- num = get_number_or_range (&state);
+ num = parser.get_number ();
if (num == 0)
warning (_("bad display number at or near '%s'"), p);
else
diff --git a/gdb/reverse.c b/gdb/reverse.c
index a1b697f..2c1abdc 100644
--- a/gdb/reverse.c
+++ b/gdb/reverse.c
@@ -216,9 +216,6 @@ delete_all_bookmarks (void)
static void
delete_bookmark_command (char *args, int from_tty)
{
- int num;
- struct get_number_or_range_state state;
-
if (bookmark_chain == NULL)
{
warning (_("No bookmarks."));
@@ -233,10 +230,10 @@ delete_bookmark_command (char *args, int from_tty)
return;
}

- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- num = get_number_or_range (&state);
+ int num = parser.get_number ();
if (!delete_one_bookmark (num))
/* Not found. */
warning (_("No bookmark #%d."), num);
@@ -323,20 +320,16 @@ bookmark_1 (int bnum)
static void
bookmarks_info (char *args, int from_tty)
{
- int bnum = -1;
-
if (!bookmark_chain)
printf_filtered (_("No bookmarks.\n"));
else if (args == NULL || *args == '\0')
bookmark_1 (-1);
else
{
- struct get_number_or_range_state state;
-
- init_number_or_range (&state, args);
- while (!state.finished)
+ number_or_range_parser parser (args);
+ while (!parser.finished ())
{
- bnum = get_number_or_range (&state);
+ int bnum = parser.get_number ();
bookmark_1 (bnum);
}
}
diff --git a/gdb/thread.c b/gdb/thread.c
index 13449a8..57b20ff 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1825,20 +1825,19 @@ thread_apply_command (char *tidlist, int from_tty)
char *cmd = NULL;
struct cleanup *old_chain;
char *saved_cmd;
- struct tid_range_parser parser;
+ tid_range_parser parser;

if (tidlist == NULL || *tidlist == '\000')
error (_("Please specify a thread ID list"));

- tid_range_parser_init (&parser, tidlist, current_inferior ()->num);
- while (!tid_range_parser_finished (&parser))
+ parser.init (tidlist, current_inferior ()->num);
+ while (!parser.finished ())
{
int inf_num, thr_start, thr_end;

- if (!tid_range_parser_get_tid_range (&parser,
- &inf_num, &thr_start, &thr_end))
+ if (!parser.get_tid_range (&inf_num, &thr_start, &thr_end))
{
- cmd = (char *) tid_range_parser_string (&parser);
+ cmd = (char *) parser.cur_tok ();
break;
}
}
@@ -1856,32 +1855,31 @@ thread_apply_command (char *tidlist, int from_tty)

make_cleanup_restore_current_thread ();

- tid_range_parser_init (&parser, tidlist, current_inferior ()->num);
- while (!tid_range_parser_finished (&parser)
- && tid_range_parser_string (&parser) < cmd)
+ parser.init (tidlist, current_inferior ()->num);
+ while (!parser.finished () && parser.cur_tok () < cmd)
{
struct thread_info *tp = NULL;
struct inferior *inf;
int inf_num, thr_num;

- tid_range_parser_get_tid (&parser, &inf_num, &thr_num);
+ parser.get_tid (&inf_num, &thr_num);
inf = find_inferior_id (inf_num);
if (inf != NULL)
tp = find_thread_id (inf, thr_num);

- if (tid_range_parser_star_range (&parser))
+ if (parser.in_star_range ())
{
if (inf == NULL)
{
warning (_("Unknown inferior %d"), inf_num);
- tid_range_parser_skip (&parser);
+ parser.skip_range ();
continue;
}

/* No use looking for threads past the highest thread number
the inferior ever had. */
if (thr_num >= inf->highest_thread_num)
- tid_range_parser_skip (&parser);
+ parser.skip_range ();

/* Be quiet about unknown threads numbers. */
if (tp == NULL)
@@ -1890,8 +1888,7 @@ thread_apply_command (char *tidlist, int from_tty)

if (tp == NULL)
{
- if (show_inferior_qualified_tids ()
- || tid_range_parser_qualified (&parser))
+ if (show_inferior_qualified_tids () || parser.tid_is_qualified ())
warning (_("Unknown thread %d.%d"), inf_num, thr_num);
else
warning (_("Unknown thread %d"), thr_num);
diff --git a/gdb/tid-parse.c b/gdb/tid-parse.c
index 68133c6..73bf673 100644
--- a/gdb/tid-parse.c
+++ b/gdb/tid-parse.c
@@ -113,29 +113,36 @@ parse_thread_id (const char *tidstr, const char **end)

/* See tid-parse.h. */

+tid_range_parser::tid_range_parser (const char *tidlist,
+ int default_inferior)
+{
+ init (tidlist, default_inferior);
+}
+
+/* See tid-parse.h. */
+
void
-tid_range_parser_init (struct tid_range_parser *parser, const char *tidlist,
- int default_inferior)
+tid_range_parser::init (const char *tidlist, int default_inferior)
{
- parser->state = TID_RANGE_STATE_INFERIOR;
- parser->string = tidlist;
- parser->inf_num = 0;
- parser->qualified = 0;
- parser->default_inferior = default_inferior;
+ m_state = STATE_INFERIOR;
+ m_cur_tok = tidlist;
+ m_inf_num = 0;
+ m_qualified = false;
+ m_default_inferior = default_inferior;
}

/* See tid-parse.h. */

-int
-tid_range_parser_finished (struct tid_range_parser *parser)
+bool
+tid_range_parser::finished () const
{
- switch (parser->state)
+ switch (m_state)
{
- case TID_RANGE_STATE_INFERIOR:
- return *parser->string == '\0';
- case TID_RANGE_STATE_THREAD_RANGE:
- case TID_RANGE_STATE_STAR_RANGE:
- return parser->range_parser.finished;
+ case STATE_INFERIOR:
+ return *m_cur_tok == '\0';
+ case STATE_THREAD_RANGE:
+ case STATE_STAR_RANGE:
+ return m_range_parser.finished ();
}

gdb_assert_not_reached (_("unhandled state"));
@@ -144,57 +151,54 @@ tid_range_parser_finished (struct tid_range_parser *parser)
/* See tid-parse.h. */

const char *
-tid_range_parser_string (struct tid_range_parser *parser)
+tid_range_parser::cur_tok () const
{
- switch (parser->state)
+ switch (m_state)
{
- case TID_RANGE_STATE_INFERIOR:
- return parser->string;
- case TID_RANGE_STATE_THREAD_RANGE:
- case TID_RANGE_STATE_STAR_RANGE:
- return parser->range_parser.string;
+ case STATE_INFERIOR:
+ return m_cur_tok;
+ case STATE_THREAD_RANGE:
+ case STATE_STAR_RANGE:
+ return m_range_parser.cur_tok ();
}

gdb_assert_not_reached (_("unhandled state"));
}

-/* See tid-parse.h. */
-
void
-tid_range_parser_skip (struct tid_range_parser *parser)
+tid_range_parser::skip_range ()
{
- gdb_assert ((parser->state == TID_RANGE_STATE_THREAD_RANGE
- || parser->state == TID_RANGE_STATE_STAR_RANGE)
- && parser->range_parser.in_range);
+ gdb_assert (m_state == STATE_THREAD_RANGE
+ || m_state == STATE_STAR_RANGE);

- tid_range_parser_init (parser, parser->range_parser.end_ptr,
- parser->default_inferior);
+ m_range_parser.skip_range ();
+ init (m_range_parser.cur_tok (), m_default_inferior);
}

/* See tid-parse.h. */

-int
-tid_range_parser_qualified (struct tid_range_parser *parser)
+bool
+tid_range_parser::tid_is_qualified () const
{
- return parser->qualified;
+ return m_qualified;
}

-/* Helper for tid_range_parser_get_tid and
- tid_range_parser_get_tid_range. Return the next range if THR_END
+/* Helper for tid_range_parser::get_tid and
+ tid_range_parser::get_tid_range. Return the next range if THR_END
is non-NULL, return a single thread ID otherwise. */

-static int
-get_tid_or_range (struct tid_range_parser *parser, int *inf_num,
- int *thr_start, int *thr_end)
+bool
+tid_range_parser::get_tid_or_range (int *inf_num,
+ int *thr_start, int *thr_end)
{
- if (parser->state == TID_RANGE_STATE_INFERIOR)
+ if (m_state == STATE_INFERIOR)
{
const char *p;
const char *space;

- space = skip_to_space (parser->string);
+ space = skip_to_space (m_cur_tok);

- p = parser->string;
+ p = m_cur_tok;
while (p < space && *p != '.')
p++;
if (p < space)
@@ -202,56 +206,53 @@ get_tid_or_range (struct tid_range_parser *parser, int *inf_num,
const char *dot = p;

/* Parse number to the left of the dot. */
- p = parser->string;
- parser->inf_num
- = get_positive_number_trailer (&p, '.', parser->string);
- if (parser->inf_num == 0)
+ p = m_cur_tok;
+ m_inf_num = get_positive_number_trailer (&p, '.', m_cur_tok);
+ if (m_inf_num == 0)
return 0;

- parser->qualified = 1;
+ m_qualified = true;
p = dot + 1;

if (isspace (*p))
- return 0;
+ return false;
}
else
{
- parser->inf_num = parser->default_inferior;
- parser->qualified = 0;
- p = parser->string;
+ m_inf_num = m_default_inferior;
+ m_qualified = false;
+ p = m_cur_tok;
}

- init_number_or_range (&parser->range_parser, p);
+ m_range_parser.init (p);
if (p[0] == '*' && (p[1] == '\0' || isspace (p[1])))
{
/* Setup the number range parser to return numbers in the
whole [1,INT_MAX] range. */
- number_range_setup_range (&parser->range_parser, 1, INT_MAX,
- skip_spaces_const (p + 1));
- parser->state = TID_RANGE_STATE_STAR_RANGE;
+ m_range_parser.setup_range (1, INT_MAX, skip_spaces_const (p + 1));
+ m_state = STATE_STAR_RANGE;
}
else
- parser->state = TID_RANGE_STATE_THREAD_RANGE;
+ m_state = STATE_THREAD_RANGE;
}

- *inf_num = parser->inf_num;
- *thr_start = get_number_or_range (&parser->range_parser);
+ *inf_num = m_inf_num;
+ *thr_start = m_range_parser.get_number ();
if (*thr_start < 0)
- error (_("negative value: %s"), parser->string);
+ error (_("negative value: %s"), m_cur_tok);
if (*thr_start == 0)
{
- parser->state = TID_RANGE_STATE_INFERIOR;
- return 0;
+ m_state = STATE_INFERIOR;
+ return false;
}

/* If we successfully parsed a thread number or finished parsing a
thread range, switch back to assuming the next TID is
inferior-qualified. */
- if (parser->range_parser.end_ptr == NULL
- || parser->range_parser.string == parser->range_parser.end_ptr)
+ if (!m_range_parser.in_range ())
{
- parser->state = TID_RANGE_STATE_INFERIOR;
- parser->string = parser->range_parser.string;
+ m_state = STATE_INFERIOR;
+ m_cur_tok = m_range_parser.cur_tok ();

if (thr_end != NULL)
*thr_end = *thr_start;
@@ -260,11 +261,12 @@ get_tid_or_range (struct tid_range_parser *parser, int *inf_num,
/* If we're midway through a range, and the caller wants the end
value, return it and skip to the end of the range. */
if (thr_end != NULL
- && (parser->state == TID_RANGE_STATE_THREAD_RANGE
- || parser->state == TID_RANGE_STATE_STAR_RANGE))
+ && (m_state == STATE_THREAD_RANGE
+ || m_state == STATE_STAR_RANGE))
{
- *thr_end = parser->range_parser.end_value;
- tid_range_parser_skip (parser);
+ *thr_end = m_range_parser.end_value ();
+
+ skip_range ();
}

return (*inf_num != 0 && *thr_start != 0);
@@ -272,32 +274,31 @@ get_tid_or_range (struct tid_range_parser *parser, int *inf_num,

/* See tid-parse.h. */

-int
-tid_range_parser_get_tid_range (struct tid_range_parser *parser, int *inf_num,
- int *thr_start, int *thr_end)
+bool
+tid_range_parser::get_tid_range (int *inf_num,
+ int *thr_start, int *thr_end)
{
gdb_assert (inf_num != NULL && thr_start != NULL && thr_end != NULL);

- return get_tid_or_range (parser, inf_num, thr_start, thr_end);
+ return get_tid_or_range (inf_num, thr_start, thr_end);
}

/* See tid-parse.h. */

-int
-tid_range_parser_get_tid (struct tid_range_parser *parser,
- int *inf_num, int *thr_num)
+bool
+tid_range_parser::get_tid (int *inf_num, int *thr_num)
{
gdb_assert (inf_num != NULL && thr_num != NULL);

- return get_tid_or_range (parser, inf_num, thr_num, NULL);
+ return get_tid_or_range (inf_num, thr_num, NULL);
}

/* See tid-parse.h. */

-int
-tid_range_parser_star_range (struct tid_range_parser *parser)
+bool
+tid_range_parser::in_star_range () const
{
- return parser->state == TID_RANGE_STATE_STAR_RANGE;
+ return m_state == STATE_STAR_RANGE;
}

/* See gdbthread.h. */
@@ -306,19 +307,16 @@ int
tid_is_in_list (const char *list, int default_inferior,
int inf_num, int thr_num)
{
- struct tid_range_parser parser;
-
if (list == NULL || *list == '\0')
return 1;

- tid_range_parser_init (&parser, list, default_inferior);
- while (!tid_range_parser_finished (&parser))
+ tid_range_parser parser (list, default_inferior);
+ while (!parser.finished ())
{
int tmp_inf, tmp_thr_start, tmp_thr_end;

- if (!tid_range_parser_get_tid_range (&parser, &tmp_inf,
- &tmp_thr_start, &tmp_thr_end))
- invalid_thread_id_error (parser.string);
+ if (!parser.get_tid_range (&tmp_inf, &tmp_thr_start, &tmp_thr_end))
+ invalid_thread_id_error (parser.cur_tok ());
if (tmp_inf == inf_num
&& tmp_thr_start <= thr_num && thr_num <= tmp_thr_end)
return 1;
diff --git a/gdb/tid-parse.h b/gdb/tid-parse.h
index 830cf36..1f6ae90 100644
--- a/gdb/tid-parse.h
+++ b/gdb/tid-parse.h
@@ -36,56 +36,6 @@ extern void ATTRIBUTE_NORETURN invalid_thread_id_error (const char *string);
thrown. */
struct thread_info *parse_thread_id (const char *tidstr, const char **end);

-/* The possible states of the tid range parser's state machine. */
-enum tid_range_state
-{
- /* Parsing the inferior number. */
- TID_RANGE_STATE_INFERIOR,
-
- /* Parsing the thread number or thread number range. */
- TID_RANGE_STATE_THREAD_RANGE,
-
- /* Parsing a star wildcard thread range. E.g., "1.*". */
- TID_RANGE_STATE_STAR_RANGE,
-};
-
-/* An object of this type is passed to tid_range_parser_get_tid. It
- must be initialized by calling tid_range_parser_init. This type is
- defined here so that it can be stack-allocated, but all members
- should be treated as opaque. */
-struct tid_range_parser
-{
- /* What sub-component are we expecting. */
- enum tid_range_state state;
-
- /* The string being parsed. When parsing has finished, this points
- past the last parsed token. */
- const char *string;
-
- /* The range parser state when we're parsing the thread number
- sub-component. */
- struct get_number_or_range_state range_parser;
-
- /* Last inferior number returned. */
- int inf_num;
-
- /* True if the TID last parsed was explicitly inferior-qualified.
- IOW, whether the spec specified an inferior number
- explicitly. */
- int qualified;
-
- /* The inferior number to assume if the TID is not qualified. */
- int default_inferior;
-};
-
-/* Initialize a tid_range_parser for use with
- tid_range_parser_get_tid. TIDLIST is the string to be parsed.
- DEFAULT_INFERIOR is the inferior number to assume if a
- non-qualified thread ID is found. */
-extern void tid_range_parser_init (struct tid_range_parser *parser,
- const char *tidlist,
- int default_inferior);
-
/* Parse a thread ID or a thread range list.

A range will be of the form
@@ -99,74 +49,133 @@ extern void tid_range_parser_init (struct tid_range_parser *parser,
<thread_number1>-<thread_number2>

in which case GDB infers the inferior number from the default
- passed to the tid_range_parser_init function.
+ passed to the constructor or to the last call to the init
+ function. */
+class tid_range_parser
+{
+public:
+ /* Default construction. Must call init before calling get_*. */
+ tid_range_parser () {}
+
+ /* Calls init automatically. See init for description of
+ parameters. */
+ tid_range_parser (const char *tidlist, int default_inferior);
+
+ /* Reinitialize a tid_range_parser. TIDLIST is the string to be
+ parsed. DEFAULT_INFERIOR is the inferior number to assume if a
+ non-qualified thread ID is found. */
+ void init (const char *tidlist, int default_inferior);

- This function is designed to be called iteratively. While
- processing a thread ID range list, at each call it will return (in
- the INF_NUM and THR_NUM output parameters) the next thread ID in
- the range (irrespective of whether the thread actually exists).
+ /* Parse a thread ID or a thread range list.

- At the beginning of parsing a thread range, the char pointer
- PARSER->string will be advanced past <thread_number1> and left
- pointing at the '-' token. Subsequent calls will not advance the
- pointer until the range is completed. The call that completes the
- range will advance the pointer past <thread_number2>.
+ This function is designed to be called iteratively. While
+ processing a thread ID range list, at each call it will return
+ (in the INF_NUM and THR_NUM output parameters) the next thread ID
+ in the range (irrespective of whether the thread actually
+ exists).

- This function advances through the input string for as long you
- call it. Once the end of the input string is reached, a call to
- tid_range_parser_finished returns false (see below).
+ At the beginning of parsing a thread range, the char pointer
+ PARSER->m_cur_tok will be advanced past <thread_number1> and left
+ pointing at the '-' token. Subsequent calls will not advance the
+ pointer until the range is completed. The call that completes
+ the range will advance the pointer past <thread_number2>.

- E.g., with list: "1.2 3.4-6":
+ This function advances through the input string for as long you
+ call it. Once the end of the input string is reached, a call to
+ finished returns false (see below).
+
+ E.g., with list: "1.2 3.4-6":

1st call: *INF_NUM=1; *THR_NUM=2 (finished==0)
2nd call: *INF_NUM=3; *THR_NUM=4 (finished==0)
3rd call: *INF_NUM=3; *THR_NUM=5 (finished==0)
4th call: *INF_NUM=3; *THR_NUM=6 (finished==1)

- Returns true if parsed a thread/range successfully, false
- otherwise. */
-extern int tid_range_parser_get_tid (struct tid_range_parser *parser,
- int *inf_num, int *thr_num);
+ Returns true if a thread/range is parsed successfully, false
+ otherwise. */
+ bool get_tid (int *inf_num, int *thr_num);

-/* Like tid_range_parser_get_tid, but return a thread ID range per
- call, rather then a single thread ID.
+ /* Like get_tid, but return a thread ID range per call, rather then
+ a single thread ID.

- If the next element in the list is a single thread ID, then
- *THR_START and *THR_END are set to the same value.
+ If the next element in the list is a single thread ID, then
+ *THR_START and *THR_END are set to the same value.

- E.g.,. with list: "1.2 3.4-6"
+ E.g.,. with list: "1.2 3.4-6"

1st call: *INF_NUM=1; *THR_START=2; *THR_END=2 (finished==0)
2nd call: *INF_NUM=3; *THR_START=4; *THR_END=6 (finished==1)

- Returns true if parsed a thread/range successfully, false
- otherwise. */
-extern int tid_range_parser_get_tid_range (struct tid_range_parser *parser,
- int *inf_num,
- int *thr_start, int *thr_end);
+ Returns true if parsed a thread/range successfully, false
+ otherwise. */
+ bool get_tid_range (int *inf_num, int *thr_start, int *thr_end);
+
+ /* Returns true if processing a star wildcard (e.g., "1.*")
+ range. */
+ bool in_star_range () const;
+
+ /* Returns true if parsing has completed. */
+ bool finished () const;
+
+ /* Return the current token being parsed. When parsing has
+ finished, this points past the last parsed token. */
+ const char *cur_tok () const;

-/* Returns non-zero if processing a star wildcard (e.g., "1.*")
- range. */
-extern int tid_range_parser_star_range (struct tid_range_parser *parser);
+ /* When parsing a range, advance past the final token in the
+ range. */
+ void skip_range ();

-/* Returns non-zero if parsing has completed. */
-extern int tid_range_parser_finished (struct tid_range_parser *parser);
+ /* True if the TID last parsed was explicitly inferior-qualified.
+ IOW, whether the spec specified an inferior number
+ explicitly. */
+ bool tid_is_qualified () const;

-/* Return the string being parsed. When parsing has finished, this
- points past the last parsed token. */
-const char *tid_range_parser_string (struct tid_range_parser *parser);
+private:
+ /* No need for these. They are intentionally not defined anywhere. */
+ tid_range_parser (const tid_range_parser &);
+ tid_range_parser &operator= (const tid_range_parser &);

-/* When parsing a range, advance past the final token in the range. */
-extern void tid_range_parser_skip (struct tid_range_parser *parser);
+ bool get_tid_or_range (int *inf_num, int *thr_start, int *thr_end);
+
+ /* The possible states of the tid range parser's state machine,
+ indicating what sub-component are we expecting. */
+ enum
+ {
+ /* Parsing the inferior number. */
+ STATE_INFERIOR,
+
+ /* Parsing the thread number or thread number range. */
+ STATE_THREAD_RANGE,
+
+ /* Parsing a star wildcard thread range. E.g., "1.*". */
+ STATE_STAR_RANGE,
+ } m_state;
+
+ /* The string being parsed. When parsing has finished, this points
+ past the last parsed token. */
+ const char *m_cur_tok;
+
+ /* The range parser state when we're parsing the thread number
+ sub-component. */
+ number_or_range_parser m_range_parser;
+
+ /* Last inferior number returned. */
+ int m_inf_num;
+
+ /* True if the TID last parsed was explicitly inferior-qualified.
+ IOW, whether the spec specified an inferior number
+ explicitly. */
+ bool m_qualified;
+
+ /* The inferior number to assume if the TID is not qualified. */
+ int m_default_inferior;
+};

-/* True if the TID last parsed was explicitly inferior-qualified.
- IOW, whether the spec specified an inferior number explicitly. */
-extern int tid_range_parser_qualified (struct tid_range_parser *parser);

/* Accept a string-form list of thread IDs such as is accepted by
- tid_range_parser_get_tid. Return true if the INF_NUM.THR.NUM
- thread is in the list. DEFAULT_INFERIOR is the inferior number to
- assume if a non-qualified thread ID is found in the list.
+ tid_range_parser. Return true if the INF_NUM.THR.NUM thread is in
+ the list. DEFAULT_INFERIOR is the inferior number to assume if a
+ non-qualified thread ID is found in the list.

By definition, an empty list includes all threads. This is to be
interpreted as typing a command such as "info threads" with no
--
2.5.5