* [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c.
2017-08-09 17:49 [PATCH v2 0/3] Some C++-ification of the FreeBSD target John Baldwin
2017-08-09 17:49 ` [PATCH v2 1/3] Fix compile in the !HAVE_KINFO_GETVMMAP case John Baldwin
2017-08-09 17:49 ` [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists John Baldwin
@ 2017-08-09 17:49 ` John Baldwin
2017-08-10 13:26 ` Pedro Alves
2 siblings, 1 reply; 9+ messages in thread
From: John Baldwin @ 2017-08-09 17:49 UTC (permalink / raw)
To: gdb-patches
- Use a custom deleter with std::unique_ptr to free() memory returned
by kinfo_getvmmap().
- Use std::string with string_printf() to generate the pathname of the
procfs 'map' file.
- Use gdb::byte_vector to manage the dynamic buffer for
TARGET_OBJECT_AUXV and the dynamically allocated array of LWP IDs.
gdb/ChangeLog:
* fbsd-nat.c [HAVE_KINFO_GETVMMAP] (struct free_deleter): New.
(fbsd_find_memory_regions): Use free_deleter with std::unique_ptr.
[!HAVE_KINFO_GETVMMAP] (fbsd_find_memory_regions): Use std::string
for `mapfilename'.
(fbsd_xfer_partial): Use gdb::byte_vector.
(fbsd_add_threads): Likewise.
---
gdb/ChangeLog | 9 +++++++++
gdb/fbsd-nat.c | 58 +++++++++++++++++++++++++++-------------------------------
2 files changed, 36 insertions(+), 31 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 133fbaf3bd..52cdc60546 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
2017-08-07 John Baldwin <jhb@FreeBSD.org>
+ * fbsd-nat.c [HAVE_KINFO_GETVMMAP] (struct free_deleter): New.
+ (fbsd_find_memory_regions): Use free_deleter with std::unique_ptr.
+ [!HAVE_KINFO_GETVMMAP] (fbsd_find_memory_regions): Use std::string
+ for `mapfilename'.
+ (fbsd_xfer_partial): Use gdb::byte_vector.
+ (fbsd_add_threads): Likewise.
+
+2017-08-07 John Baldwin <jhb@FreeBSD.org>
+
* fbsd-nat.c: [!HAVE_KINFO_GETVMMAP]: Include <sys/user.h> and
"filestuff.h".
(fbsd_find_memory_regions): Fix `mapfile' initialization.
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 3d3aa3df59..721e72b85c 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -18,6 +18,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "defs.h"
+#include "byte-vector.h"
#include "gdbcore.h"
#include "inferior.h"
#include "regcache.h"
@@ -75,6 +76,14 @@ fbsd_pid_to_exec_file (struct target_ops *self, int pid)
}
#ifdef HAVE_KINFO_GETVMMAP
+/* Deleter for std::unique_ptr that invokes free. */
+
+template <typename T>
+struct free_deleter
+{
+ void operator() (T *ptr) const { free (ptr); }
+};
+
/* Iterate over all the memory regions in the current inferior,
calling FUNC for each memory region. OBFD is passed as the last
argument to FUNC. */
@@ -84,20 +93,17 @@ fbsd_find_memory_regions (struct target_ops *self,
find_memory_region_ftype func, void *obfd)
{
pid_t pid = ptid_get_pid (inferior_ptid);
- struct kinfo_vmentry *vmentl, *kve;
+ struct kinfo_vmentry *kve;
uint64_t size;
- struct cleanup *cleanup;
int i, nitems;
- vmentl = kinfo_getvmmap (pid, &nitems);
+ std::unique_ptr<struct kinfo_vmentry, free_deleter<struct kinfo_vmentry>>
+ vmentl (kinfo_getvmmap (pid, &nitems));
if (vmentl == NULL)
perror_with_name (_("Couldn't fetch VM map entries."));
- cleanup = make_cleanup (free, vmentl);
- for (i = 0; i < nitems; i++)
+ for (i = 0, kve = vmentl.get (); i < nitems; i++, kve++)
{
- kve = &vmentl[i];
-
/* Skip unreadable segments and those where MAP_NOCORE has been set. */
if (!(kve->kve_protection & KVME_PROT_READ)
|| kve->kve_flags & KVME_FLAG_NOCOREDUMP)
@@ -128,7 +134,6 @@ fbsd_find_memory_regions (struct target_ops *self,
kve->kve_protection & KVME_PROT_WRITE,
kve->kve_protection & KVME_PROT_EXEC, 1, obfd);
}
- do_cleanups (cleanup);
return 0;
}
#else
@@ -162,21 +167,18 @@ fbsd_find_memory_regions (struct target_ops *self,
find_memory_region_ftype func, void *obfd)
{
pid_t pid = ptid_get_pid (inferior_ptid);
- char *mapfilename;
unsigned long start, end, size;
char protection[4];
int read, write, exec;
- struct cleanup *cleanup;
- mapfilename = xstrprintf ("/proc/%ld/map", (long) pid);
- cleanup = make_cleanup (xfree, mapfilename);
- gdb_file_up mapfile (fopen (mapfilename, "r"));
+ std::string mapfilename = string_printf ("/proc/%ld/map", (long) pid);
+ gdb_file_up mapfile (fopen (mapfilename.c_str (), "r"));
if (mapfile == NULL)
- error (_("Couldn't open %s."), mapfilename);
+ error (_("Couldn't open %s."), mapfilename.c_str ());
if (info_verbose)
fprintf_filtered (gdb_stdout,
- "Reading memory regions from %s\n", mapfilename);
+ "Reading memory regions from %s\n", mapfilename.c_str ());
/* Now iterate until end-of-file. */
while (fbsd_read_mapping (mapfile.get (), &start, &end, &protection[0]))
@@ -202,7 +204,6 @@ fbsd_find_memory_regions (struct target_ops *self,
func (start, size, read, write, exec, 1, obfd);
}
- do_cleanups (cleanup);
return 0;
}
#endif
@@ -392,8 +393,8 @@ fbsd_xfer_partial (struct target_ops *ops, enum target_object object,
#endif
case TARGET_OBJECT_AUXV:
{
- struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
- unsigned char *buf;
+ gdb::byte_vector buf_storage;
+ gdb_byte *buf;
size_t buflen;
int mib[4];
@@ -411,8 +412,8 @@ fbsd_xfer_partial (struct target_ops *ops, enum target_object object,
else
{
buflen = offset + len;
- buf = XCNEWVEC (unsigned char, buflen);
- cleanup = make_cleanup (xfree, buf);
+ buf_storage.resize (buflen);
+ buf = buf_storage.data ();
}
if (sysctl (mib, 4, buf, &buflen, NULL, 0) == 0)
{
@@ -426,11 +427,9 @@ fbsd_xfer_partial (struct target_ops *ops, enum target_object object,
else
buflen = 0;
}
- do_cleanups (cleanup);
*xfered_len = buflen;
return (buflen == 0) ? TARGET_XFER_EOF : TARGET_XFER_OK;
}
- do_cleanups (cleanup);
return TARGET_XFER_E_IO;
}
default:
@@ -624,8 +623,6 @@ fbsd_enable_proc_events (pid_t pid)
static void
fbsd_add_threads (pid_t pid)
{
- struct cleanup *cleanup;
- lwpid_t *lwps;
int i, nlwps;
gdb_assert (!in_thread_list (pid_to_ptid (pid)));
@@ -633,16 +630,16 @@ fbsd_add_threads (pid_t pid)
if (nlwps == -1)
perror_with_name (("ptrace"));
- lwps = XCNEWVEC (lwpid_t, nlwps);
- cleanup = make_cleanup (xfree, lwps);
+ gdb::unique_xmalloc_ptr<lwpid_t> lwps (XCNEWVEC (lwpid_t, nlwps));
- nlwps = ptrace (PT_GETLWPLIST, pid, (caddr_t) lwps, nlwps);
+ nlwps = ptrace (PT_GETLWPLIST, pid, (caddr_t) lwps.get (), nlwps);
if (nlwps == -1)
perror_with_name (("ptrace"));
for (i = 0; i < nlwps; i++)
{
- ptid_t ptid = ptid_build (pid, lwps[i], 0);
+ lwpid_t lwp = lwps.get ()[i];
+ ptid_t ptid = ptid_build (pid, lwp, 0);
if (!in_thread_list (ptid))
{
@@ -651,7 +648,7 @@ fbsd_add_threads (pid_t pid)
/* Don't add exited threads. Note that this is only called
when attaching to a multi-threaded process. */
- if (ptrace (PT_LWPINFO, lwps[i], (caddr_t) &pl, sizeof pl) == -1)
+ if (ptrace (PT_LWPINFO, lwp, (caddr_t) &pl, sizeof pl) == -1)
perror_with_name (("ptrace"));
if (pl.pl_flags & PL_FLAG_EXITED)
continue;
@@ -659,11 +656,10 @@ fbsd_add_threads (pid_t pid)
if (debug_fbsd_lwp)
fprintf_unfiltered (gdb_stdlog,
"FLWP: adding thread for LWP %u\n",
- lwps[i]);
+ lwp);
add_thread (ptid);
}
}
- do_cleanups (cleanup);
}
/* Implement the "to_update_thread_list" target_ops method. */
--
2.13.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 0/3] Some C++-ification of the FreeBSD target
@ 2017-08-09 17:49 John Baldwin
2017-08-09 17:49 ` [PATCH v2 1/3] Fix compile in the !HAVE_KINFO_GETVMMAP case John Baldwin
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: John Baldwin @ 2017-08-09 17:49 UTC (permalink / raw)
To: gdb-patches
Just a few C++ cleanups of the FreeBSD native target. This should remove
the remaining cleanups from the BSD targets other than bsd-uthread.c.
Changes since V1 are to address feedback from Simon using gdb::byte_vector
instead of gdb::unique_xmalloc_ptr in one place and forward_list instead
of list in one case.
John Baldwin (3):
Fix compile in the !HAVE_KINFO_GETVMMAP case.
Replace remaining cleanups in fbsd-nat.c.
Replace home-grown linked-lists in FreeBSD's native target with STL
lists.
gdb/ChangeLog | 26 +++++++++++
gdb/fbsd-nat.c | 133 ++++++++++++++++++++++-----------------------------------
2 files changed, 77 insertions(+), 82 deletions(-)
--
2.13.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.
2017-08-09 17:49 [PATCH v2 0/3] Some C++-ification of the FreeBSD target John Baldwin
2017-08-09 17:49 ` [PATCH v2 1/3] Fix compile in the !HAVE_KINFO_GETVMMAP case John Baldwin
@ 2017-08-09 17:49 ` John Baldwin
2017-08-09 18:16 ` Simon Marchi
2017-08-09 17:49 ` [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c John Baldwin
2 siblings, 1 reply; 9+ messages in thread
From: John Baldwin @ 2017-08-09 17:49 UTC (permalink / raw)
To: gdb-patches
FreeBSD's native target uses linked-lists to keep track of pending fork
events and fake vfork done events. Replace the first list with std::list
and the second with std::forward_list.
gdb/ChangeLog:
* fbsd-nat.c (struct fbsd_fork_info): Remove.
(fbsd_pending_children): Use std::list.
(fbsd_remember_child): Likewise.
(fbsd_is_child_pending): Likewise.
(fbsd_pending_vfork_done): Use std::forward_list.
(fbsd_add_vfork_done): Likewise.
(fbsd_is_vfork_done_pending): Likewise.
(fbsd_next_vfork_done): Likewise.
---
gdb/ChangeLog | 11 +++++++++
gdb/fbsd-nat.c | 71 +++++++++++++++++-----------------------------------------
2 files changed, 32 insertions(+), 50 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 52cdc60546..eb2ea389df 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@
2017-08-07 John Baldwin <jhb@FreeBSD.org>
+ * fbsd-nat.c (struct fbsd_fork_info): Remove.
+ (fbsd_pending_children): Use std::list.
+ (fbsd_remember_child): Likewise.
+ (fbsd_is_child_pending): Likewise.
+ (fbsd_pending_vfork_done): Use std::forward_list.
+ (fbsd_add_vfork_done): Likewise.
+ (fbsd_is_vfork_done_pending): Likewise.
+ (fbsd_next_vfork_done): Likewise.
+
+2017-08-07 John Baldwin <jhb@FreeBSD.org>
+
* fbsd-nat.c [HAVE_KINFO_GETVMMAP] (struct free_deleter): New.
(fbsd_find_memory_regions): Use free_deleter with std::unique_ptr.
[!HAVE_KINFO_GETVMMAP] (fbsd_find_memory_regions): Use std::string
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 721e72b85c..c89343a24f 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -41,6 +41,8 @@
#include "elf-bfd.h"
#include "fbsd-nat.h"
+#include <list>
+
/* Return the name of a file that can be opened to get the symbols for
the child process identified by PID. */
@@ -712,13 +714,7 @@ fbsd_update_thread_list (struct target_ops *ops)
sake. FreeBSD versions newer than 9.1 contain both fixes.
*/
-struct fbsd_fork_info
-{
- struct fbsd_fork_info *next;
- ptid_t ptid;
-};
-
-static struct fbsd_fork_info *fbsd_pending_children;
+static std::list<ptid_t> fbsd_pending_children;
/* Record a new child process event that is reported before the
corresponding fork event in the parent. */
@@ -726,11 +722,7 @@ static struct fbsd_fork_info *fbsd_pending_children;
static void
fbsd_remember_child (ptid_t pid)
{
- struct fbsd_fork_info *info = XCNEW (struct fbsd_fork_info);
-
- info->ptid = pid;
- info->next = fbsd_pending_children;
- fbsd_pending_children = info;
+ fbsd_pending_children.push_front (pid);
}
/* Check for a previously-recorded new child process event for PID.
@@ -739,39 +731,26 @@ fbsd_remember_child (ptid_t pid)
static ptid_t
fbsd_is_child_pending (pid_t pid)
{
- struct fbsd_fork_info *info, *prev;
- ptid_t ptid;
-
- prev = NULL;
- for (info = fbsd_pending_children; info; prev = info, info = info->next)
- {
- if (ptid_get_pid (info->ptid) == pid)
- {
- if (prev == NULL)
- fbsd_pending_children = info->next;
- else
- prev->next = info->next;
- ptid = info->ptid;
- xfree (info);
- return ptid;
- }
- }
+ for (auto it = fbsd_pending_children.begin ();
+ it != fbsd_pending_children.end (); it++)
+ if (it->pid () == pid)
+ {
+ ptid_t ptid = *it;
+ fbsd_pending_children.erase (it);
+ return ptid;
+ }
return null_ptid;
}
#ifndef PTRACE_VFORK
-static struct fbsd_fork_info *fbsd_pending_vfork_done;
+static std::forward_list<ptid_t> fbsd_pending_vfork_done;
/* Record a pending vfork done event. */
static void
fbsd_add_vfork_done (ptid_t pid)
{
- struct fbsd_fork_info *info = XCNEW (struct fbsd_fork_info);
-
- info->ptid = pid;
- info->next = fbsd_pending_vfork_done;
- fbsd_pending_vfork_done = info;
+ fbsd_pending_vfork_done.push_front (pid);
}
/* Check for a pending vfork done event for a specific PID. */
@@ -779,13 +758,10 @@ fbsd_add_vfork_done (ptid_t pid)
static int
fbsd_is_vfork_done_pending (pid_t pid)
{
- struct fbsd_fork_info *info;
-
- for (info = fbsd_pending_vfork_done; info != NULL; info = info->next)
- {
- if (ptid_get_pid (info->ptid) == pid)
- return 1;
- }
+ for (auto it = fbsd_pending_vfork_done.begin ();
+ it != fbsd_pending_vfork_done.end (); it++)
+ if (it->pid () == pid)
+ return 1;
return 0;
}
@@ -795,15 +771,10 @@ fbsd_is_vfork_done_pending (pid_t pid)
static ptid_t
fbsd_next_vfork_done (void)
{
- struct fbsd_fork_info *info;
- ptid_t ptid;
-
- if (fbsd_pending_vfork_done != NULL)
+ if (!fbsd_pending_vfork_done.empty ())
{
- info = fbsd_pending_vfork_done;
- fbsd_pending_vfork_done = info->next;
- ptid = info->ptid;
- xfree (info);
+ ptid_t ptid = fbsd_pending_vfork_done.front ();
+ fbsd_pending_vfork_done.pop_front ();
return ptid;
}
return null_ptid;
--
2.13.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] Fix compile in the !HAVE_KINFO_GETVMMAP case.
2017-08-09 17:49 [PATCH v2 0/3] Some C++-ification of the FreeBSD target John Baldwin
@ 2017-08-09 17:49 ` John Baldwin
2017-08-09 17:49 ` [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists John Baldwin
2017-08-09 17:49 ` [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c John Baldwin
2 siblings, 0 replies; 9+ messages in thread
From: John Baldwin @ 2017-08-09 17:49 UTC (permalink / raw)
To: gdb-patches
gdb/ChangeLog:
* fbsd-nat.c: [!HAVE_KINFO_GETVMMAP]: Include <sys/user.h> and
"filestuff.h".
(fbsd_find_memory_regions): Fix `mapfile' initialization.
---
gdb/ChangeLog | 6 ++++++
gdb/fbsd-nat.c | 6 ++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c4ad2bf1eb..133fbaf3bd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2017-08-07 John Baldwin <jhb@FreeBSD.org>
+
+ * fbsd-nat.c: [!HAVE_KINFO_GETVMMAP]: Include <sys/user.h> and
+ "filestuff.h".
+ (fbsd_find_memory_regions): Fix `mapfile' initialization.
+
2017-08-07 Maciej W. Rozycki <macro@imgtec.com>
PR breakpoints/21886
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 833f460237..3d3aa3df59 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -30,9 +30,11 @@
#include <sys/ptrace.h>
#include <sys/signal.h>
#include <sys/sysctl.h>
-#ifdef HAVE_KINFO_GETVMMAP
#include <sys/user.h>
+#ifdef HAVE_KINFO_GETVMMAP
#include <libutil.h>
+#else
+#include "filestuff.h"
#endif
#include "elf-bfd.h"
@@ -168,7 +170,7 @@ fbsd_find_memory_regions (struct target_ops *self,
mapfilename = xstrprintf ("/proc/%ld/map", (long) pid);
cleanup = make_cleanup (xfree, mapfilename);
- gdb_file_up mapfile = fopen (mapfilename, "r");
+ gdb_file_up mapfile (fopen (mapfilename, "r"));
if (mapfile == NULL)
error (_("Couldn't open %s."), mapfilename);
--
2.13.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.
2017-08-09 17:49 ` [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists John Baldwin
@ 2017-08-09 18:16 ` Simon Marchi
2017-08-09 19:15 ` John Baldwin
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2017-08-09 18:16 UTC (permalink / raw)
To: John Baldwin; +Cc: gdb-patches
On 2017-08-09 19:47, John Baldwin wrote:
> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index 721e72b85c..c89343a24f 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -41,6 +41,8 @@
> #include "elf-bfd.h"
> #include "fbsd-nat.h"
>
> +#include <list>
> +
> /* Return the name of a file that can be opened to get the symbols for
> the child process identified by PID. */
>
> @@ -712,13 +714,7 @@ fbsd_update_thread_list (struct target_ops *ops)
> sake. FreeBSD versions newer than 9.1 contain both fixes.
> */
>
> -struct fbsd_fork_info
> -{
> - struct fbsd_fork_info *next;
> - ptid_t ptid;
> -};
> -
> -static struct fbsd_fork_info *fbsd_pending_children;
> +static std::list<ptid_t> fbsd_pending_children;
Can't this be a forward_list as well?
The series LGTM otherwise.
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.
2017-08-09 18:16 ` Simon Marchi
@ 2017-08-09 19:15 ` John Baldwin
2017-08-09 19:31 ` Simon Marchi
0 siblings, 1 reply; 9+ messages in thread
From: John Baldwin @ 2017-08-09 19:15 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On Wednesday, August 09, 2017 08:16:29 PM Simon Marchi wrote:
> On 2017-08-09 19:47, John Baldwin wrote:
> > diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> > index 721e72b85c..c89343a24f 100644
> > --- a/gdb/fbsd-nat.c
> > +++ b/gdb/fbsd-nat.c
> > @@ -41,6 +41,8 @@
> > #include "elf-bfd.h"
> > #include "fbsd-nat.h"
> >
> > +#include <list>
> > +
> > /* Return the name of a file that can be opened to get the symbols for
> > the child process identified by PID. */
> >
> > @@ -712,13 +714,7 @@ fbsd_update_thread_list (struct target_ops *ops)
> > sake. FreeBSD versions newer than 9.1 contain both fixes.
> > */
> >
> > -struct fbsd_fork_info
> > -{
> > - struct fbsd_fork_info *next;
> > - ptid_t ptid;
> > -};
> > -
> > -static struct fbsd_fork_info *fbsd_pending_children;
> > +static std::list<ptid_t> fbsd_pending_children;
>
> Can't this be a forward_list as well?
Not trivially. fbsd_is_child_pending has to walk the list looking for a
specific PID and remove that specific list entry (not always the first
entry). Right now this is using the following loop:
for (auto it = fbsd_pending_children.begin ();
it != fbsd_pending_children.end (); it++)
if (it->pid () == pid)
{
ptid_t ptid = *it;
fbsd_pending_children.erase (it);
return ptid;
}
return null_ptid;
I'm not sure if it's legal to do something like this for a forward_list:
for (auto it = fbsd_pending_children.before_begin ();
it + 1 != fbsd_pending_children.end (); it++)
if ((it + 1)->pid () == pid)
{
ptid_t ptid = *(it + 1);
fbsd_pending_childern.erase_after (it);
return ptid;
}
return null_ptid;
Even if it is legal, I'm not sure it is more readable. These lists should
generally be quite small, so I think readability is more important than
optimization in this case.
--
John Baldwin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.
2017-08-09 19:15 ` John Baldwin
@ 2017-08-09 19:31 ` Simon Marchi
0 siblings, 0 replies; 9+ messages in thread
From: Simon Marchi @ 2017-08-09 19:31 UTC (permalink / raw)
To: John Baldwin; +Cc: gdb-patches
On 2017-08-09 21:15, John Baldwin wrote:
> On Wednesday, August 09, 2017 08:16:29 PM Simon Marchi wrote:
>> On 2017-08-09 19:47, John Baldwin wrote:
>> > diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
>> > index 721e72b85c..c89343a24f 100644
>> > --- a/gdb/fbsd-nat.c
>> > +++ b/gdb/fbsd-nat.c
>> > @@ -41,6 +41,8 @@
>> > #include "elf-bfd.h"
>> > #include "fbsd-nat.h"
>> >
>> > +#include <list>
>> > +
>> > /* Return the name of a file that can be opened to get the symbols for
>> > the child process identified by PID. */
>> >
>> > @@ -712,13 +714,7 @@ fbsd_update_thread_list (struct target_ops *ops)
>> > sake. FreeBSD versions newer than 9.1 contain both fixes.
>> > */
>> >
>> > -struct fbsd_fork_info
>> > -{
>> > - struct fbsd_fork_info *next;
>> > - ptid_t ptid;
>> > -};
>> > -
>> > -static struct fbsd_fork_info *fbsd_pending_children;
>> > +static std::list<ptid_t> fbsd_pending_children;
>>
>> Can't this be a forward_list as well?
>
> Not trivially. fbsd_is_child_pending has to walk the list looking for
> a
> specific PID and remove that specific list entry (not always the first
> entry). Right now this is using the following loop:
>
> for (auto it = fbsd_pending_children.begin ();
> it != fbsd_pending_children.end (); it++)
> if (it->pid () == pid)
> {
> ptid_t ptid = *it;
> fbsd_pending_children.erase (it);
> return ptid;
> }
> return null_ptid;
>
> I'm not sure if it's legal to do something like this for a
> forward_list:
>
> for (auto it = fbsd_pending_children.before_begin ();
> it + 1 != fbsd_pending_children.end (); it++)
> if ((it + 1)->pid () == pid)
> {
> ptid_t ptid = *(it + 1);
> fbsd_pending_childern.erase_after (it);
> return ptid;
> }
> return null_ptid;
>
> Even if it is legal, I'm not sure it is more readable. These lists
> should
> generally be quite small, so I think readability is more important than
> optimization in this case.
Ah, ok. I thought it could be done trivially, but I can't easily
build-test on FreeBSD right now.
I agree with you.
Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c.
2017-08-09 17:49 ` [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c John Baldwin
@ 2017-08-10 13:26 ` Pedro Alves
2017-08-10 14:51 ` John Baldwin
0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2017-08-10 13:26 UTC (permalink / raw)
To: John Baldwin, gdb-patches
On 08/09/2017 06:47 PM, John Baldwin wrote:
> + gdb::unique_xmalloc_ptr<lwpid_t> lwps (XCNEWVEC (lwpid_t, nlwps));
> + lwpid_t lwp = lwps.get ()[i];
I was going to say:
~~~
Note that std::unique_ptr has an array version that avoids the
get() in "lwps.get ()[i]". I.e., with:
gdb::unique_xmalloc_ptr<lwpid_t[]> lwps (XCNEWVEC (lwpid_t, nlwps));
^^
you can then write the more natural:
lwpid_t lwp = lwps[i];
~~~
... but realized it doesn't work currently, because I missed adding
an xfree_deleter array specialization. Fixed now:
https://sourceware.org/ml/gdb-patches/2017-08/msg00212.html
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c.
2017-08-10 13:26 ` Pedro Alves
@ 2017-08-10 14:51 ` John Baldwin
0 siblings, 0 replies; 9+ messages in thread
From: John Baldwin @ 2017-08-10 14:51 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Thursday, August 10, 2017 02:24:52 PM Pedro Alves wrote:
> On 08/09/2017 06:47 PM, John Baldwin wrote:
> > + gdb::unique_xmalloc_ptr<lwpid_t> lwps (XCNEWVEC (lwpid_t, nlwps));
>
> > + lwpid_t lwp = lwps.get ()[i];
>
> I was going to say:
>
> ~~~
> Note that std::unique_ptr has an array version that avoids the
> get() in "lwps.get ()[i]". I.e., with:
>
> gdb::unique_xmalloc_ptr<lwpid_t[]> lwps (XCNEWVEC (lwpid_t, nlwps));
> ^^
> you can then write the more natural:
>
> lwpid_t lwp = lwps[i];
> ~~~
>
> ... but realized it doesn't work currently, because I missed adding
> an xfree_deleter array specialization. Fixed now:
>
> https://sourceware.org/ml/gdb-patches/2017-08/msg00212.html
Thanks, I'll change this to use that instead.
--
John Baldwin
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-08-10 14:51 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-09 17:49 [PATCH v2 0/3] Some C++-ification of the FreeBSD target John Baldwin
2017-08-09 17:49 ` [PATCH v2 1/3] Fix compile in the !HAVE_KINFO_GETVMMAP case John Baldwin
2017-08-09 17:49 ` [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists John Baldwin
2017-08-09 18:16 ` Simon Marchi
2017-08-09 19:15 ` John Baldwin
2017-08-09 19:31 ` Simon Marchi
2017-08-09 17:49 ` [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c John Baldwin
2017-08-10 13:26 ` Pedro Alves
2017-08-10 14:51 ` John Baldwin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).