[PATCH v2 0/3] Some C++-ification of the FreeBSD target

[PATCH v2 0/3] Some C++-ification of the FreeBSD target

[John Baldwin]

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

[PATCH v2 1/3] Fix compile in the !HAVE_KINFO_GETVMMAP case.

[John Baldwin]

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

[PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.

[John Baldwin]

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

Re: [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.

[Simon Marchi]

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

Re: [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.

[John Baldwin]

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

Re: [PATCH v2 3/3] Replace home-grown linked-lists in FreeBSD's native target with STL lists.

[Simon Marchi]

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

[PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c.

[John Baldwin]

- 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

Re: [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c.

[Pedro Alves]

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

Re: [PATCH v2 2/3] Replace remaining cleanups in fbsd-nat.c.

[John Baldwin]

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