This is the mail archive of the
gdb-cvs@sourceware.org
mailing list for the GDB project.
[binutils-gdb] remote: C++ify thread_item and threads_listing_context
- From: Simon Marchi <simark at sourceware dot org>
- To: gdb-cvs at sourceware dot org
- Date: 24 Nov 2017 15:44:05 -0000
- Subject: [binutils-gdb] remote: C++ify thread_item and threads_listing_context
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=21fe1c752e254167d953fa8c846280f63a3a5290
commit 21fe1c752e254167d953fa8c846280f63a3a5290
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri Nov 24 10:40:13 2017 -0500
remote: C++ify thread_item and threads_listing_context
This patch C++ifies the thread_item and threads_listing_context
structures in remote.c. thread_item::{extra,name} are changed to
std::string. As a result, there's a bit of awkwardness in
remote_update_thread_list, where we have to xstrdup those strings when
filling the private_thread_info structure. This is removed in the
following patch, where private_thread_info is also C++ified and its
corresponding fields made std::string too. The xstrdup then becomes an
std::move.
Other than that there's nothing really special, it's a usual day-to-day
VEC -> vector and char* -> std::string change. It allows removing a
cleanup in remote_update_thread_list.
Note that an overload of hex2bin that returns a gdb::byte_vector is
added, with corresponding selftests.
gdb/ChangeLog:
* remote.c (struct thread_item): Add constructor, disable copy
construction and copy assignment, define default move
construction and move assignment.
<extra, name>: Change type to std::string.
<core>: Initialize.
<thread_handle>: Make non-pointer.
(thread_item_t): Remove typedef.
(DEF_VEC_O(thread_item_t)): Remove.
(threads_listing_context) <contains_thread>: New method.
<remove_thread>: New method.
<items>: Change type to std::vector.
(clear_threads_listing_context): Remove.
(threads_listing_context_remove): Remove.
(remote_newthread_step): Use thread_item constructor, adjust to
change to std::vector.
(start_thread): Use thread_item constructor, adjust to change to
std::vector.
(end_thread): Adjust to change to std::vector and std::string.
(remote_get_threads_with_qthreadinfo): Use thread_item
constructor, adjust to std::vector.
(remote_update_thread_list): Adjust to change to std::vector and
std::string, use threads_listing_context methods.
(remove_child_of_pending_fork): Adjust.
(remove_new_fork_children): Adjust.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add rsp-low-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add rsp-low-selftests.o.
* unittests/rsp-low-selftests.c: New file.
* common/rsp-low.h: Include common/byte-vector.h.
(hex2bin): New overload.
* common/rsp-low.c (hex2bin): New overload.
Diff:
---
gdb/ChangeLog | 33 +++++++
gdb/Makefile.in | 2 +
gdb/common/rsp-low.c | 13 +++
gdb/common/rsp-low.h | 6 ++
gdb/remote.c | 197 +++++++++++++++-----------------------
gdb/unittests/rsp-low-selftests.c | 59 ++++++++++++
6 files changed, 189 insertions(+), 121 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b0356db..7165ec3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,36 @@
+2017-11-24 Simon Marchi <simon.marchi@polymtl.ca>
+
+ * remote.c (struct thread_item): Add constructor, disable copy
+ construction and copy assignment, define default move
+ construction and move assignment.
+ <extra, name>: Change type to std::string.
+ <core>: Initialize.
+ <thread_handle>: Make non-pointer.
+ (thread_item_t): Remove typedef.
+ (DEF_VEC_O(thread_item_t)): Remove.
+ (threads_listing_context) <contains_thread>: New method.
+ <remove_thread>: New method.
+ <items>: Change type to std::vector.
+ (clear_threads_listing_context): Remove.
+ (threads_listing_context_remove): Remove.
+ (remote_newthread_step): Use thread_item constructor, adjust to
+ change to std::vector.
+ (start_thread): Use thread_item constructor, adjust to change to
+ std::vector.
+ (end_thread): Adjust to change to std::vector and std::string.
+ (remote_get_threads_with_qthreadinfo): Use thread_item
+ constructor, adjust to std::vector.
+ (remote_update_thread_list): Adjust to change to std::vector and
+ std::string, use threads_listing_context methods.
+ (remove_child_of_pending_fork): Adjust.
+ (remove_new_fork_children): Adjust.
+ * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add rsp-low-selftests.c.
+ (SUBDIR_UNITTESTS_OBS): Add rsp-low-selftests.o.
+ * unittests/rsp-low-selftests.c: New file.
+ * common/rsp-low.h: Include common/byte-vector.h.
+ (hex2bin): New overload.
+ * common/rsp-low.c (hex2bin): New overload.
+
2017-11-24 Simon Marchi <simon.marchi@ericsson.com>
* inferior.h (private_inferior): Define structure type, add
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index ec9900c..7198556 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -539,6 +539,7 @@ SUBDIR_UNITTESTS_SRCS = \
unittests/offset-type-selftests.c \
unittests/optional-selftests.c \
unittests/ptid-selftests.c \
+ unittests/rsp-low-selftests.c \
unittests/scoped_restore-selftests.c \
unittests/xml-utils-selftests.c
@@ -553,6 +554,7 @@ SUBDIR_UNITTESTS_OBS = \
offset-type-selftests.o \
optional-selftests.o \
ptid-selftests.o \
+ rsp-low-selftests.o \
scoped_restore-selftests.o \
xml-utils-selftests.o
diff --git a/gdb/common/rsp-low.c b/gdb/common/rsp-low.c
index 85987f7..9948fbb 100644
--- a/gdb/common/rsp-low.c
+++ b/gdb/common/rsp-low.c
@@ -132,6 +132,19 @@ hex2bin (const char *hex, gdb_byte *bin, int count)
/* See rsp-low.h. */
+gdb::byte_vector
+hex2bin (const char *hex)
+{
+ size_t bin_len = strlen (hex) / 2;
+ gdb::byte_vector bin (bin_len);
+
+ hex2bin (hex, bin.data (), bin_len);
+
+ return bin;
+}
+
+/* See rsp-low.h. */
+
std::string
hex2str (const char *hex)
{
diff --git a/gdb/common/rsp-low.h b/gdb/common/rsp-low.h
index 99dc93f..039f19c 100644
--- a/gdb/common/rsp-low.h
+++ b/gdb/common/rsp-low.h
@@ -20,6 +20,8 @@
#ifndef COMMON_RSP_LOW_H
#define COMMON_RSP_LOW_H
+#include "common/byte-vector.h"
+
/* Convert hex digit A to a number, or throw an exception. */
extern int fromhex (int a);
@@ -52,6 +54,10 @@ extern const char *unpack_varlen_hex (const char *buff, ULONGEST *result);
extern int hex2bin (const char *hex, gdb_byte *bin, int count);
+/* Like the above, but return a gdb::byte_vector. */
+
+gdb::byte_vector hex2bin (const char *hex);
+
/* Like hex2bin, but return a std::string. */
extern std::string hex2str (const char *hex);
diff --git a/gdb/remote.c b/gdb/remote.c
index 5445bb2..8c76b7e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2972,25 +2972,33 @@ remote_threadlist_iterator (rmt_thread_action stepfunction, void *context,
/* A thread found on the remote target. */
-typedef struct thread_item
+struct thread_item
{
+ explicit thread_item (ptid_t ptid_)
+ : ptid (ptid_)
+ {}
+
+ thread_item (thread_item &&other) = default;
+ thread_item &operator= (thread_item &&other) = default;
+
+ DISABLE_COPY_AND_ASSIGN (thread_item);
+
/* The thread's PTID. */
ptid_t ptid;
- /* The thread's extra info. May be NULL. */
- char *extra;
+ /* The thread's extra info. */
+ std::string extra;
- /* The thread's name. May be NULL. */
- char *name;
+ /* The thread's name. */
+ std::string name;
/* The core the thread was running on. -1 if not known. */
- int core;
+ int core = -1;
/* The thread handle associated with the thread. */
- gdb::byte_vector *thread_handle;
+ gdb::byte_vector thread_handle;
-} thread_item_t;
-DEF_VEC_O(thread_item_t);
+};
/* Context passed around to the various methods listing remote
threads. As new threads are found, they're added to the ITEMS
@@ -2998,66 +3006,54 @@ DEF_VEC_O(thread_item_t);
struct threads_listing_context
{
- /* The threads found on the remote target. */
- VEC (thread_item_t) *items;
-};
+ /* Return true if this object contains an entry for a thread with ptid
+ PTID. */
-/* Discard the contents of the constructed thread listing context. */
+ bool contains_thread (ptid_t ptid) const
+ {
+ auto match_ptid = [&] (const thread_item &item)
+ {
+ return item.ptid == ptid;
+ };
-static void
-clear_threads_listing_context (void *p)
-{
- struct threads_listing_context *context
- = (struct threads_listing_context *) p;
- int i;
- struct thread_item *item;
+ auto it = std::find_if (this->items.begin (),
+ this->items.end (),
+ match_ptid);
- for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
- {
- xfree (item->extra);
- xfree (item->name);
- delete item->thread_handle;
- }
+ return it != this->items.end ();
+ }
- VEC_free (thread_item_t, context->items);
-}
+ /* Remove the thread with ptid PTID. */
-/* Remove the thread specified as the related_pid field of WS
- from the CONTEXT list. */
+ void remove_thread (ptid_t ptid)
+ {
+ auto match_ptid = [&] (const thread_item &item)
+ {
+ return item.ptid == ptid;
+ };
-static void
-threads_listing_context_remove (struct target_waitstatus *ws,
- struct threads_listing_context *context)
-{
- struct thread_item *item;
- int i;
- ptid_t child_ptid = ws->value.related_pid;
+ auto it = std::remove_if (this->items.begin (),
+ this->items.end (),
+ match_ptid);
- for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i)
- {
- if (ptid_equal (item->ptid, child_ptid))
- {
- VEC_ordered_remove (thread_item_t, context->items, i);
- break;
- }
- }
-}
+ if (it != this->items.end ())
+ this->items.erase (it);
+ }
+
+ /* The threads found on the remote target. */
+ std::vector<thread_item> items;
+};
static int
remote_newthread_step (threadref *ref, void *data)
{
struct threads_listing_context *context
= (struct threads_listing_context *) data;
- struct thread_item item;
- int pid = ptid_get_pid (inferior_ptid);
-
- item.ptid = ptid_build (pid, threadref_to_int (ref), 0);
- item.core = -1;
- item.name = NULL;
- item.extra = NULL;
- item.thread_handle = nullptr;
+ int pid = inferior_ptid.pid ();
+ int lwp = threadref_to_int (ref);
+ ptid_t ptid (pid, lwp);
- VEC_safe_push (thread_item_t, context->items, &item);
+ context->items.emplace_back (ptid);
return 1; /* continue iterator */
}
@@ -3109,37 +3105,25 @@ start_thread (struct gdb_xml_parser *parser,
{
struct threads_listing_context *data
= (struct threads_listing_context *) user_data;
-
- struct thread_item item;
- char *id;
struct gdb_xml_value *attr;
- id = (char *) xml_find_attribute (attributes, "id")->value;
- item.ptid = read_ptid (id, NULL);
+ char *id = (char *) xml_find_attribute (attributes, "id")->value;
+ ptid_t ptid = read_ptid (id, NULL);
+
+ data->items.emplace_back (ptid);
+ thread_item &item = data->items.back ();
attr = xml_find_attribute (attributes, "core");
if (attr != NULL)
item.core = *(ULONGEST *) attr->value;
- else
- item.core = -1;
attr = xml_find_attribute (attributes, "name");
- item.name = attr != NULL ? xstrdup ((const char *) attr->value) : NULL;
+ if (attr != NULL)
+ item.name = (const char *) attr->value;
attr = xml_find_attribute (attributes, "handle");
if (attr != NULL)
- {
- item.thread_handle = new gdb::byte_vector
- (strlen ((const char *) attr->value) / 2);
- hex2bin ((const char *) attr->value, item.thread_handle->data (),
- item.thread_handle->size ());
- }
- else
- item.thread_handle = nullptr;
-
- item.extra = 0;
-
- VEC_safe_push (thread_item_t, data->items, &item);
+ item.thread_handle = hex2bin ((const char *) attr->value);
}
static void
@@ -3150,8 +3134,8 @@ end_thread (struct gdb_xml_parser *parser,
struct threads_listing_context *data
= (struct threads_listing_context *) user_data;
- if (body_text && *body_text)
- VEC_last (thread_item_t, data->items)->extra = xstrdup (body_text);
+ if (body_text != NULL && *body_text != '\0')
+ data->items.back ().extra = body_text;
}
const struct gdb_xml_attribute thread_attributes[] = {
@@ -3227,15 +3211,8 @@ remote_get_threads_with_qthreadinfo (struct target_ops *ops,
{
do
{
- struct thread_item item;
-
- item.ptid = read_ptid (bufp, &bufp);
- item.core = -1;
- item.name = NULL;
- item.extra = NULL;
- item.thread_handle = nullptr;
-
- VEC_safe_push (thread_item_t, context->items, &item);
+ ptid_t ptid = read_ptid (bufp, &bufp);
+ context->items.emplace_back (ptid);
}
while (*bufp++ == ','); /* comma-separated list */
putpkt ("qsThreadInfo");
@@ -3261,12 +3238,8 @@ static void
remote_update_thread_list (struct target_ops *ops)
{
struct threads_listing_context context;
- struct cleanup *old_chain;
int got_list = 0;
- context.items = NULL;
- old_chain = make_cleanup (clear_threads_listing_context, &context);
-
/* We have a few different mechanisms to fetch the thread list. Try
them all, starting with the most preferred one first, falling
back to older methods. */
@@ -3275,12 +3248,11 @@ remote_update_thread_list (struct target_ops *ops)
|| remote_get_threads_with_ql (ops, &context))
{
int i;
- struct thread_item *item;
struct thread_info *tp, *tmp;
got_list = 1;
- if (VEC_empty (thread_item_t, context.items)
+ if (context.items.empty ()
&& remote_thread_always_alive (ops, inferior_ptid))
{
/* Some targets don't really support threads, but still
@@ -3288,7 +3260,6 @@ remote_update_thread_list (struct target_ops *ops)
listing packets, instead of replying "packet not
supported". Exit early so we don't delete the main
thread. */
- do_cleanups (old_chain);
return;
}
@@ -3297,15 +3268,7 @@ remote_update_thread_list (struct target_ops *ops)
target. */
ALL_THREADS_SAFE (tp, tmp)
{
- for (i = 0;
- VEC_iterate (thread_item_t, context.items, i, item);
- ++i)
- {
- if (ptid_equal (item->ptid, tp->ptid))
- break;
- }
-
- if (i == VEC_length (thread_item_t, context.items))
+ if (!context.contains_thread (tp->ptid))
{
/* Not found. */
delete_thread (tp->ptid);
@@ -3318,11 +3281,9 @@ remote_update_thread_list (struct target_ops *ops)
remove_new_fork_children (&context);
/* And now add threads we don't know about yet to our list. */
- for (i = 0;
- VEC_iterate (thread_item_t, context.items, i, item);
- ++i)
+ for (thread_item &item : context.items)
{
- if (!ptid_equal (item->ptid, null_ptid))
+ if (item.ptid != null_ptid)
{
struct private_thread_info *info;
/* In non-stop mode, we assume new found threads are
@@ -3331,16 +3292,14 @@ remote_update_thread_list (struct target_ops *ops)
stopped. */
int executing = target_is_non_stop_p () ? 1 : 0;
- remote_notice_new_inferior (item->ptid, executing);
+ remote_notice_new_inferior (item.ptid, executing);
- info = get_private_info_ptid (item->ptid);
- info->core = item->core;
- info->extra = item->extra;
- item->extra = NULL;
- info->name = item->name;
- item->name = NULL;
- info->thread_handle = item->thread_handle;
- item->thread_handle = nullptr;
+ info = get_private_info_ptid (item.ptid);
+ info->core = item.core;
+ info->extra = xstrdup (item.extra.c_str ());
+ info->name = xstrdup (item.name.c_str ());
+ info->thread_handle
+ = new gdb::byte_vector (std::move (item.thread_handle));
}
}
}
@@ -3353,8 +3312,6 @@ remote_update_thread_list (struct target_ops *ops)
no-op. See remote_thread_alive. */
prune_threads ();
}
-
- do_cleanups (old_chain);
}
/*
@@ -6521,7 +6478,7 @@ remove_child_of_pending_fork (QUEUE (stop_reply_p) *q,
if (event->ws.kind == TARGET_WAITKIND_FORKED
|| event->ws.kind == TARGET_WAITKIND_VFORKED
|| event->ws.kind == TARGET_WAITKIND_THREAD_EXITED)
- threads_listing_context_remove (&event->ws, context);
+ context->remove_thread (event->ws.value.related_pid);
return 1;
}
@@ -6547,9 +6504,7 @@ remove_new_fork_children (struct threads_listing_context *context)
struct target_waitstatus *ws = thread_pending_fork_status (thread);
if (is_pending_fork_parent (ws, pid, thread->ptid))
- {
- threads_listing_context_remove (ws, context);
- }
+ context->remove_thread (ws->value.related_pid);
}
/* Check for any pending fork events (not reported or processed yet)
diff --git a/gdb/unittests/rsp-low-selftests.c b/gdb/unittests/rsp-low-selftests.c
new file mode 100644
index 0000000..e20fedf
--- /dev/null
+++ b/gdb/unittests/rsp-low-selftests.c
@@ -0,0 +1,59 @@
+/* Unit tests for the rsp-low.c file.
+
+ Copyright (C) 2017 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#include "defs.h"
+#include "selftest.h"
+#include "common/rsp-low.h"
+
+namespace selftests {
+namespace rsp_low {
+
+/* Test the variant of hex2bin that returns a byte_vector. */
+
+static void test_hex2bin_byte_vector ()
+{
+ gdb::byte_vector bv;
+
+ /* Test an empty string. */
+ bv = hex2bin ("");
+ SELF_CHECK (bv.size () == 0);
+
+ /* Test a well-formated hex string. */
+ bv = hex2bin ("abcd01");
+ SELF_CHECK (bv.size () == 3);
+ SELF_CHECK (bv[0] == 0xab);
+ SELF_CHECK (bv[1] == 0xcd);
+ SELF_CHECK (bv[2] == 0x01);
+
+ /* Test an odd-length hex string. */
+ bv = hex2bin ("0123c");
+ SELF_CHECK (bv.size () == 2);
+ SELF_CHECK (bv[0] == 0x01);
+ SELF_CHECK (bv[1] == 0x23);
+}
+
+} /* namespace rsp_low */
+} /* namespace selftests */
+
+void
+_initialize_rsp_low_selftests ()
+{
+ selftests::register_test ("hex2bin_byte_vector",
+ selftests::rsp_low::test_hex2bin_byte_vector);
+}