[PATCH 0/4] Make "checkpoint" not rely on inferior_ptid

[PATCH 0/4] Make "checkpoint" not rely on inferior_ptid

[Pedro Alves]

Patch #1 is a patch that I had on the multi-target branch.  Patches
#2-#4 are follow up C++ifications/cleanups while I was at it.

Pedro Alves (4):
  Make "checkpoint" not rely on inferior_ptid
  C++ify fork_info, use std::list
  linux-fork.c: rewrite inf_has_multiple_threads
  Eliminate fork_info::clobber_regs

 gdb/linux-fork.c | 377 ++++++++++++++++++++++++-------------------------------
 gdb/linux-fork.h |   2 +-
 2 files changed, 163 insertions(+), 216 deletions(-)

-- 
2.14.4

[PATCH 2/4] C++ify fork_info, use std::list

[Pedro Alves]

- Convert new_fork and free_fork to fork_info ctor/dtor.
- Use std::list.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linux-fork.c: Include <list>.
	(fork_list): Now a std::list instance.
	(fork_info): Add ctor, dtor, and in-class initialize all fields.
	(forks_exist_p, find_last_fork): Adjust.
	(new_fork): Delete.
	(one_fork_p): New.
	(add_fork): Adjust.
	(free_fork): Delete, folded into fork_info::~fork_info().
	(delete_fork, find_fork_ptid, find_fork_id, find_fork_pid):
	Adjust.
	(init_fork_list): Delete.
	(linux_fork_killall, linux_fork_mourn_inferior)
	(linux_fork_detach, info_checkpoints_command): Adjust.
	(_initialize_linux_fork): No longer call init_fork_list.
---
 gdb/linux-fork.c | 261 +++++++++++++++++++++++--------------------------------
 1 file changed, 108 insertions(+), 153 deletions(-)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index a748deaa3a..1c7b7ca123 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -35,30 +35,66 @@
 #include <dirent.h>
 #include <ctype.h>
 
-struct fork_info *fork_list;
-static int highest_fork_num;
+#include <list>
 
 /* Fork list data structure:  */
 struct fork_info
 {
-  struct fork_info *next;
-  ptid_t ptid;
-  ptid_t parent_ptid;
-  int num;			/* Convenient handle (GDB fork id).  */
-  readonly_detached_regcache *savedregs;	/* Convenient for info fork, saves
-				   having to actually switch contexts.  */
-  CORE_ADDR pc;
-  int clobber_regs;		/* True if we should restore saved regs.  */
-  off_t *filepos;		/* Set of open file descriptors' offsets.  */
-  int maxfd;
+  explicit fork_info (pid_t pid)
+    : ptid (pid, pid, 0)
+  {
+  }
+
+  ~fork_info ()
+  {
+    /* Notes on step-resume breakpoints: since this is a concern for
+       threads, let's convince ourselves that it's not a concern for
+       forks.  There are two ways for a fork_info to be created.
+       First, by the checkpoint command, in which case we're at a gdb
+       prompt and there can't be any step-resume breakpoint.  Second,
+       by a fork in the user program, in which case we *may* have
+       stepped into the fork call, but regardless of whether we follow
+       the parent or the child, we will return to the same place and
+       the step-resume breakpoint, if any, will take care of itself as
+       usual.  And unlike threads, we do not save a private copy of
+       the step-resume breakpoint -- so we're OK.  */
+
+    if (savedregs)
+      delete savedregs;
+    if (filepos)
+      xfree (filepos);
+  }
+
+  ptid_t ptid = null_ptid;
+  ptid_t parent_ptid = null_ptid;
+
+  /* Convenient handle (GDB fork id).  */
+  int num = 0;
+
+  /* Convenient for info fork, saves having to actually switch
+     contexts.  */
+  readonly_detached_regcache *savedregs = nullptr;
+
+  CORE_ADDR pc = 0;
+
+  /* True if we should restore saved regs.  */
+  int clobber_regs = 0;
+
+  /* Set of open file descriptors' offsets.  */
+  off_t *filepos = nullptr;
+
+  int maxfd = 0;
 };
 
+static std::list<fork_info> fork_list;
+static int highest_fork_num;
+
 /* Fork list methods:  */
 
 int
 forks_exist_p (void)
 {
-  return (fork_list != NULL);
+  return !fork_list.empty ();
 }
 
 /* Return the last fork in the list.  */
@@ -66,26 +102,19 @@ forks_exist_p (void)
 static struct fork_info *
 find_last_fork (void)
 {
-  struct fork_info *last;
-
-  if (fork_list == NULL)
+  if (fork_list.empty ())
     return NULL;
 
-  for (last = fork_list; last->next != NULL; last = last->next)
-    ;
-  return last;
+  return &fork_list.back ();
 }
 
-/* Allocate a new fork.  */
+/* Return true iff there's one fork in the list.  */
 
-static struct fork_info *
-new_fork (pid_t pid)
+static bool
+one_fork_p ()
 {
-  struct fork_info *fp;
-
-  fp = XCNEW (struct fork_info);
-  fp->ptid = ptid_t (pid, pid, 0);
-  return fp;
+  return (!fork_list.empty ()
+	  && &fork_list.front () == &fork_list.back ());
 }
 
 /* Add a new fork to the internal fork list.  */
@@ -93,93 +122,46 @@ new_fork (pid_t pid)
 void
 add_fork (pid_t pid)
 {
-  struct fork_info *fp = new_fork (pid);
-
-  if (fork_list == NULL)
-    {
-      fork_list = fp;
-      highest_fork_num = 0;
-    }
-  else
-    {
-      struct fork_info *last = find_last_fork ();
+  fork_list.emplace_back (pid);
 
-      last->next = fp;
-    }
+  if (one_fork_p ())
+    highest_fork_num = 0;
 
+  fork_info *fp = &fork_list.back ();
   fp->num = ++highest_fork_num;
 }
 
-static void
-free_fork (struct fork_info *fp)
-{
-  /* Notes on step-resume breakpoints: since this is a concern for
-     threads, let's convince ourselves that it's not a concern for
-     forks.  There are two ways for a fork_info to be created.  First,
-     by the checkpoint command, in which case we're at a gdb prompt
-     and there can't be any step-resume breakpoint.  Second, by a fork
-     in the user program, in which case we *may* have stepped into the
-     fork call, but regardless of whether we follow the parent or the
-     child, we will return to the same place and the step-resume
-     breakpoint, if any, will take care of itself as usual.  And
-     unlike threads, we do not save a private copy of the step-resume
-     breakpoint -- so we're OK.  */
-
-  if (fp)
-    {
-      if (fp->savedregs)
-	delete fp->savedregs;
-      if (fp->filepos)
-	xfree (fp->filepos);
-      xfree (fp);
-    }
-}
-
 static void
 delete_fork (ptid_t ptid)
 {
-  struct fork_info *fp, *fpprev;
-
-  fpprev = NULL;
-
   linux_target->low_forget_process (ptid.pid ());
 
-  for (fp = fork_list; fp; fpprev = fp, fp = fp->next)
-    if (fp->ptid == ptid)
-      break;
-
-  if (!fp)
-    return;
-
-  if (fpprev)
-    fpprev->next = fp->next;
-  else
-    fork_list = fp->next;
-
-  free_fork (fp);
+  for (auto it = fork_list.begin (); it != fork_list.end (); ++it)
+    if (it->ptid == ptid)
+      {
+	fork_list.erase (it);
 
-  /* Special case: if there is now only one process in the list,
-     and if it is (hopefully!) the current inferior_ptid, then
-     remove it, leaving the list empty -- we're now down to the
-     default case of debugging a single process.  */
-  if (fork_list != NULL && fork_list->next == NULL &&
-      fork_list->ptid == inferior_ptid)
-    {
-      /* Last fork -- delete from list and handle as solo process
-	 (should be a safe recursion).  */
-      delete_fork (inferior_ptid);
-    }
+	/* Special case: if there is now only one process in the list,
+	   and if it is (hopefully!) the current inferior_ptid, then
+	   remove it, leaving the list empty -- we're now down to the
+	   default case of debugging a single process.  */
+	if (one_fork_p () && fork_list.front ().ptid == inferior_ptid)
+	  {
+	    /* Last fork -- delete from list and handle as solo
+	       process (should be a safe recursion).  */
+	    delete_fork (inferior_ptid);
+	  }
+	return;
+      }
 }
 
 /* Find a fork_info by matching PTID.  */
 static struct fork_info *
 find_fork_ptid (ptid_t ptid)
 {
-  struct fork_info *fp;
-
-  for (fp = fork_list; fp; fp = fp->next)
-    if (fp->ptid == ptid)
-      return fp;
+  for (fork_info &fi : fork_list)
+    if (fi.ptid == ptid)
+      return &fi;
 
   return NULL;
 }
@@ -188,11 +170,9 @@ find_fork_ptid (ptid_t ptid)
 static struct fork_info *
 find_fork_id (int num)
 {
-  struct fork_info *fp;
-
-  for (fp = fork_list; fp; fp = fp->next)
-    if (fp->num == num)
-      return fp;
+  for (fork_info &fi : fork_list)
+    if (fi.num == num)
+      return &fi;
 
   return NULL;
 }
@@ -201,11 +181,9 @@ find_fork_id (int num)
 extern struct fork_info *
 find_fork_pid (pid_t pid)
 {
-  struct fork_info *fp;
-
-  for (fp = fork_list; fp; fp = fp->next)
-    if (pid == fp->ptid.pid ())
-      return fp;
+  for (fork_info &fi : fork_list)
+    if (pid == fi.ptid.pid ())
+      return &fi;
 
   return NULL;
 }
@@ -220,23 +198,6 @@ fork_id_to_ptid (int num)
     return ptid_t (-1);
 }
 
-static void
-init_fork_list (void)
-{
-  struct fork_info *fp, *fpnext;
-
-  if (!fork_list)
-    return;
-
-  for (fp = fork_list; fp; fp = fpnext)
-    {
-      fpnext = fp->next;
-      free_fork (fp);
-    }
-
-  fork_list = NULL;
-}
-
 /* Fork list <-> gdb interface.  */
 
 /* Utility function for fork_load/fork_save.
@@ -350,13 +311,12 @@ linux_fork_killall (void)
      status for it) -- however any process may be a child
      or a parent, so may get a SIGCHLD from a previously
      killed child.  Wait them all out.  */
-  struct fork_info *fp;
-  pid_t pid, ret;
-  int status;
 
-  for (fp = fork_list; fp; fp = fp->next)
+  for (fork_info &fi : fork_list)
     {
-      pid = fp->ptid.pid ();
+      pid_t pid = fi.ptid.pid ();
+      int status;
+      pid_t ret;
       do {
 	/* Use SIGKILL instead of PTRACE_KILL because the former works even
 	   if the thread is running, while the later doesn't.  */
@@ -367,7 +327,9 @@ linux_fork_killall (void)
 	 died.  MVS comment cut-and-pasted from linux-nat.  */
       } while (ret == pid && WIFSTOPPED (status));
     }
-  init_fork_list ();	/* Clear list, prepare to start fresh.  */
+
+  /* Clear list, prepare to start fresh.  */
+  fork_list.clear ();
 }
 
 /* The current inferior_ptid has exited, but there are other viable
@@ -394,7 +356,7 @@ linux_fork_mourn_inferior (void)
   /* There should still be a fork - if there's only one left,
      delete_fork won't remove it, because we haven't updated
      inferior_ptid yet.  */
-  gdb_assert (fork_list);
+  gdb_assert (!fork_list.empty ());
 
   last = find_last_fork ();
   fork_load_infrun_state (last);
@@ -402,7 +364,7 @@ linux_fork_mourn_inferior (void)
 		   target_pid_to_str (inferior_ptid));
 
   /* If there's only one fork, switch back to non-fork mode.  */
-  if (fork_list->next == NULL)
+  if (one_fork_p ())
     delete_fork (inferior_ptid);
 }
 
@@ -425,16 +387,16 @@ linux_fork_detach (int from_tty)
   /* There should still be a fork - if there's only one left,
      delete_fork won't remove it, because we haven't updated
      inferior_ptid yet.  */
-  gdb_assert (fork_list);
+  gdb_assert (!fork_list.empty ());
 
-  fork_load_infrun_state (fork_list);
+  fork_load_infrun_state (&fork_list.front ());
 
   if (from_tty)
     printf_filtered (_("[Switching to %s]\n"),
 		     target_pid_to_str (inferior_ptid));
 
   /* If there's only one fork, switch back to non-fork mode.  */
-  if (fork_list->next == NULL)
+  if (one_fork_p ())
     delete_fork (inferior_ptid);
 }
 
@@ -606,34 +568,31 @@ static void
 info_checkpoints_command (const char *arg, int from_tty)
 {
   struct gdbarch *gdbarch = get_current_arch ();
-  struct symtab_and_line sal;
-  struct fork_info *fp;
-  ULONGEST pc;
   int requested = -1;
-  struct fork_info *printed = NULL;
+  const fork_info *printed = NULL;
 
   if (arg && *arg)
     requested = (int) parse_and_eval_long (arg);
 
-  for (fp = fork_list; fp; fp = fp->next)
+  for (const fork_info &fi : fork_list)
     {
-      if (requested > 0 && fp->num != requested)
+      if (requested > 0 && fi.num != requested)
 	continue;
 
-      printed = fp;
-      if (fp->ptid == inferior_ptid)
+      printed = &fi;
+      if (fi.ptid == inferior_ptid)
 	printf_filtered ("* ");
       else
 	printf_filtered ("  ");
 
-      pc = fp->pc;
-      printf_filtered ("%d %s", fp->num, target_pid_to_str (fp->ptid));
-      if (fp->num == 0)
+      ULONGEST pc = fi.pc;
+      printf_filtered ("%d %s", fi.num, target_pid_to_str (fi.ptid));
+      if (fi.num == 0)
 	printf_filtered (_(" (main process)"));
       printf_filtered (_(" at "));
       fputs_filtered (paddress (gdbarch, pc), gdb_stdout);
 
-      sal = find_pc_line (pc, 0);
+      symtab_and_line sal = find_pc_line (pc, 0);
       if (sal.symtab)
 	printf_filtered (_(", file %s"),
 			 symtab_to_filename_for_display (sal.symtab));
@@ -762,14 +721,12 @@ checkpoint_command (const char *args, int from_tty)
   if (!fp)
     error (_("Failed to find new fork"));
 
-  if (fork_list->next == NULL)
+  if (one_fork_p ())
     {
       /* Special case -- if this is the first fork in the list (the
 	 list was hitherto empty), then add inferior_ptid first, as a
 	 special zeroeth fork id.  */
-      fork_info *first = new_fork (inferior_ptid.pid ());
-      first->next = fork_list;
-      fork_list = first;
+      fork_list.emplace_front (inferior_ptid.pid ());
     }
 
   fork_save_infrun_state (fp, 1);
@@ -816,8 +773,6 @@ restart_command (const char *args, int from_tty)
 void
 _initialize_linux_fork (void)
 {
-  init_fork_list ();
-
   /* Checkpoint command: create a fork of the inferior process
      and set it aside for later debugging.  */
 
-- 
2.14.4

[PATCH 3/4] linux-fork.c: rewrite inf_has_multiple_threads

[Pedro Alves]

There's no need to iterate over all threads of all inferiors here.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linux-fork.c (inf_has_multiple_thread_cb): Delete.
	(inf_has_multiple_threads): Return 'bool' and rewrite using
	inferior_info::threads().
---
 gdb/linux-fork.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 1c7b7ca123..4bc454ba6d 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -627,31 +627,20 @@ linux_fork_checkpointing_p (int pid)
   return (checkpointing_pid == pid);
 }
 
-/* Callback for iterate over threads.  Used to check whether
-   the current inferior is multi-threaded.  Returns true as soon
-   as it sees the second thread of the current inferior.  */
-
-static int
-inf_has_multiple_thread_cb (struct thread_info *tp, void *data)
-{
-  int *count_p = (int *) data;
-  
-  if (current_inferior ()->pid == tp->ptid.pid ())
-    (*count_p)++;
-  
-  /* Stop the iteration if multiple threads have been detected.  */
-  return *count_p > 1;
-}
-
 /* Return true if the current inferior is multi-threaded.  */
 
-static int
-inf_has_multiple_threads (void)
+static bool
+inf_has_multiple_threads ()
 {
   int count = 0;
 
-  iterate_over_threads (inf_has_multiple_thread_cb, &count);
-  return (count > 1);
+  /* Return true as soon as we see the second thread of the current
+     inferior.  */
+  for (thread_info *tp ATTRIBUTE_UNUSED : current_inferior ()->threads ())
+    if (++count > 1)
+      return true;
+
+  return false;
 }
 
 static void
-- 
2.14.4

[PATCH 4/4] Eliminate fork_info::clobber_regs

[Pedro Alves]

All fork_save_infrun_state callers pass '1' as CLOBBER_REGS nowadays.

The larger hunk in fork_save_infrun_state is just a reindentation.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linux-fork.c (fork_info::clobber_regs): Delete.
	(fork_load_infrun_state): Remove reference to 'clobber_regs'.
	(fork_save_infrun_state): Remove 'clobber_regs' parameter.  Update
	comment.  Adjust.
	(scoped_switch_fork_info::scoped_switch_fork_info)
	(checkpoint_command, linux_fork_context): Adjust
	fork_save_infrun_state calls.
---
 gdb/linux-fork.c | 79 +++++++++++++++++++++++++-------------------------------
 1 file changed, 35 insertions(+), 44 deletions(-)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 4bc454ba6d..51e8fddbc2 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -77,9 +77,6 @@ struct fork_info
 
   CORE_ADDR pc = 0;
 
-  /* True if we should restore saved regs.  */
-  int clobber_regs = 0;
-
   /* Set of open file descriptors' offsets.  */
   off_t *filepos = nullptr;
 
@@ -223,7 +220,7 @@ fork_load_infrun_state (struct fork_info *fp)
 
   linux_nat_switch_fork (fp->ptid);
 
-  if (fp->savedregs && fp->clobber_regs)
+  if (fp->savedregs)
     get_current_regcache ()->restore (fp->savedregs);
 
   registers_changed ();
@@ -245,11 +242,10 @@ fork_load_infrun_state (struct fork_info *fp)
     }
 }
 
-/* Save infrun state for the fork PTID.
-   Exported for use by linux child_follow_fork.  */
+/* Save infrun state for the fork FP.  */
 
 static void
-fork_save_infrun_state (struct fork_info *fp, int clobber_regs)
+fork_save_infrun_state (struct fork_info *fp)
 {
   char path[PATH_MAX];
   struct dirent *de;
@@ -260,44 +256,39 @@ fork_save_infrun_state (struct fork_info *fp, int clobber_regs)
 
   fp->savedregs = new readonly_detached_regcache (*get_current_regcache ());
   fp->pc = regcache_read_pc (get_current_regcache ());
-  fp->clobber_regs = clobber_regs;
 
-  if (clobber_regs)
+  /* Now save the 'state' (file position) of all open file descriptors.
+     Unfortunately fork does not take care of that for us...  */
+  snprintf (path, PATH_MAX, "/proc/%ld/fd", (long) fp->ptid.pid ());
+  if ((d = opendir (path)) != NULL)
     {
-      /* Now save the 'state' (file position) of all open file descriptors.
-	 Unfortunately fork does not take care of that for us...  */
-      snprintf (path, PATH_MAX, "/proc/%ld/fd",
-		(long) fp->ptid.pid ());
-      if ((d = opendir (path)) != NULL)
+      long tmp;
+
+      fp->maxfd = 0;
+      while ((de = readdir (d)) != NULL)
 	{
-	  long tmp;
-
-	  fp->maxfd = 0;
-	  while ((de = readdir (d)) != NULL)
-	    {
-	      /* Count open file descriptors (actually find highest
-		 numbered).  */
-	      tmp = strtol (&de->d_name[0], NULL, 10);
-	      if (fp->maxfd < tmp)
-		fp->maxfd = tmp;
-	    }
-	  /* Allocate array of file positions.  */
-	  fp->filepos = XRESIZEVEC (off_t, fp->filepos, fp->maxfd + 1);
-
-	  /* Initialize to -1 (invalid).  */
-	  for (tmp = 0; tmp <= fp->maxfd; tmp++)
-	    fp->filepos[tmp] = -1;
-
-	  /* Now find actual file positions.  */
-	  rewinddir (d);
-	  while ((de = readdir (d)) != NULL)
-	    if (isdigit (de->d_name[0]))
-	      {
-		tmp = strtol (&de->d_name[0], NULL, 10);
-		fp->filepos[tmp] = call_lseek (tmp, 0, SEEK_CUR);
-	      }
-	  closedir (d);
+	  /* Count open file descriptors (actually find highest
+	     numbered).  */
+	  tmp = strtol (&de->d_name[0], NULL, 10);
+	  if (fp->maxfd < tmp)
+	    fp->maxfd = tmp;
 	}
+      /* Allocate array of file positions.  */
+      fp->filepos = XRESIZEVEC (off_t, fp->filepos, fp->maxfd + 1);
+
+      /* Initialize to -1 (invalid).  */
+      for (tmp = 0; tmp <= fp->maxfd; tmp++)
+	fp->filepos[tmp] = -1;
+
+      /* Now find actual file positions.  */
+      rewinddir (d);
+      while ((de = readdir (d)) != NULL)
+	if (isdigit (de->d_name[0]))
+	  {
+	    tmp = strtol (&de->d_name[0], NULL, 10);
+	    fp->filepos[tmp] = call_lseek (tmp, 0, SEEK_CUR);
+	  }
+      closedir (d);
     }
 }
 
@@ -421,7 +412,7 @@ public:
 	gdb_assert (m_oldfp != nullptr);
 	newfp = find_fork_ptid (pptid);
 	gdb_assert (newfp != nullptr);
-	fork_save_infrun_state (m_oldfp, 1);
+	fork_save_infrun_state (m_oldfp);
 	remove_breakpoints ();
 	fork_load_infrun_state (newfp);
 	insert_breakpoints ();
@@ -718,7 +709,7 @@ checkpoint_command (const char *args, int from_tty)
       fork_list.emplace_front (inferior_ptid.pid ());
     }
 
-  fork_save_infrun_state (fp, 1);
+  fork_save_infrun_state (fp);
   fp->parent_ptid = last_target_ptid;
 }
 
@@ -733,7 +724,7 @@ linux_fork_context (struct fork_info *newfp, int from_tty)
   oldfp = find_fork_ptid (inferior_ptid);
   gdb_assert (oldfp != NULL);
 
-  fork_save_infrun_state (oldfp, 1);
+  fork_save_infrun_state (oldfp);
   remove_breakpoints ();
   fork_load_infrun_state (newfp);
   insert_breakpoints ();
-- 
2.14.4

[PATCH 1/4] Make "checkpoint" not rely on inferior_ptid

[Pedro Alves]

Don't rely on "inferior_ptid" deep within add_fork.  In the
multi-target branch, I'm forcing inferior_ptid to null_ptid early in
infrun event handling to make sure we inadvertently rely on the
current thread/target when we shouldn't, and that caught some bad
or unnecessary assumptions throughout.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* linux-fork.c (new_fork): New, split out of ...
	(add_fork): ... this.  Return void.  Move "first fork" special
	case from here, to ...
	(checkpoint_command): ... here.
	* linux-linux.h (add_fork): Return void.
---
 gdb/linux-fork.c | 44 ++++++++++++++++++++++++++++----------------
 gdb/linux-fork.h |  2 +-
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index b1b390c5c6..a748deaa3a 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -76,29 +76,30 @@ find_last_fork (void)
   return last;
 }
 
-/* Add a fork to the internal fork list.  */
+/* Allocate a new fork.  */
 
-struct fork_info *
-add_fork (pid_t pid)
+static struct fork_info *
+new_fork (pid_t pid)
 {
   struct fork_info *fp;
 
-  if (fork_list == NULL && pid != inferior_ptid.pid ())
-    {
-      /* Special case -- if this is the first fork in the list
-	 (the list is hitherto empty), and if this new fork is
-	 NOT the current inferior_ptid, then add inferior_ptid
-	 first, as a special zeroeth fork id.  */
-      highest_fork_num = -1;
-      add_fork (inferior_ptid.pid ());	/* safe recursion */
-    }
-
   fp = XCNEW (struct fork_info);
   fp->ptid = ptid_t (pid, pid, 0);
-  fp->num = ++highest_fork_num;
+  return fp;
+}
+
+/* Add a new fork to the internal fork list.  */
+
+void
+add_fork (pid_t pid)
+{
+  struct fork_info *fp = new_fork (pid);
 
   if (fork_list == NULL)
-    fork_list = fp;
+    {
+      fork_list = fp;
+      highest_fork_num = 0;
+    }
   else
     {
       struct fork_info *last = find_last_fork ();
@@ -106,7 +107,7 @@ add_fork (pid_t pid)
       last->next = fp;
     }
 
-  return fp;
+  fp->num = ++highest_fork_num;
 }
 
 static void
@@ -760,6 +761,17 @@ checkpoint_command (const char *args, int from_tty)
 
   if (!fp)
     error (_("Failed to find new fork"));
+
+  if (fork_list->next == NULL)
+    {
+      /* Special case -- if this is the first fork in the list (the
+	 list was hitherto empty), then add inferior_ptid first, as a
+	 special zeroeth fork id.  */
+      fork_info *first = new_fork (inferior_ptid.pid ());
+      first->next = fork_list;
+      fork_list = first;
+    }
+
   fork_save_infrun_state (fp, 1);
   fp->parent_ptid = last_target_ptid;
 }
diff --git a/gdb/linux-fork.h b/gdb/linux-fork.h
index e6f130438c..f918bcb3d9 100644
--- a/gdb/linux-fork.h
+++ b/gdb/linux-fork.h
@@ -21,7 +21,7 @@
 #define LINUX_FORK_H
 
 struct fork_info;
-extern struct fork_info *add_fork (pid_t);
+extern void add_fork (pid_t);
 extern struct fork_info *find_fork_pid (pid_t);
 extern void linux_fork_killall (void);
 extern void linux_fork_mourn_inferior (void);
-- 
2.14.4

Re: [PATCH 1/4] Make "checkpoint" not rely on inferior_ptid

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> In the
Pedro> multi-target branch, I'm forcing inferior_ptid to null_ptid early in
Pedro> infrun event handling to make sure we inadvertently rely on the
Pedro> current thread/target when we shouldn't, and that caught some bad
Pedro> or unnecessary assumptions throughout.

This read a bit strangely to me.  Maybe instead write "to make sure we
don't inadvertently rely on the current thread/target,..."?

The patch itself seemed fine to me.  Big +1 to reducing the impact of
globals.

Tom

Re: [PATCH 1/4] Make "checkpoint" not rely on inferior_ptid

[Pedro Alves]

On 03/05/2019 07:32 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> In the
> Pedro> multi-target branch, I'm forcing inferior_ptid to null_ptid early in
> Pedro> infrun event handling to make sure we inadvertently rely on the
> Pedro> current thread/target when we shouldn't, and that caught some bad
> Pedro> or unnecessary assumptions throughout.
> 
> This read a bit strangely to me.  Maybe instead write "to make sure we
> don't inadvertently rely on the current thread/target,..."?
> 

Oh, yes, I missed typing the "don't".  Thanks for spotting that.

> The patch itself seemed fine to me.  Big +1 to reducing the impact of
> globals.
Thanks,
Pedro Alves

Re: [PATCH 0/4] Make "checkpoint" not rely on inferior_ptid

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Patch #1 is a patch that I had on the multi-target branch.  Patches
Pedro> #2-#4 are follow up C++ifications/cleanups while I was at it.

I read through all of them and they seem fine to me.
Thanks for doing this.

Tom