Re: [PATCH] GDB checkpoint can't/shouldn't be possible with multiple threads on Linux

[Kevin Pouget <kevin.pouget@gmail.com>]

everything should have been fixed now,


> Can you have a dialogue with your mailer, convincing it
> to attach text files as Content-Type: text/$SOMETHING
> rather than application/octet-stream + base64 ?
> It's more steps to to read and harder to quote-for-review
> binary attachments than should be necessary.  Thanks.

gmail is my provider, not easy to talk to them; copy-and-paste should be better


> > +  gdb_assert(current_inferior() != NULL);
>
> (Many other things would break if this wasn't true.)

that was a redundant verification before checking
> > +  if (!target_has_execution) error (_("The program is not being run."));
so it's now removed

> Space before `;' again.  Let's reserve variables named
> `has_FOO' for booleans.  Otherwise, the code is harder
> to read (the "if (has_multiple_threads > 1)" check below
> made me go "hmm?").  You could encapsulate the magic count
> in a predicate function:
>
> static int
> inf_has_multiple_threads (void)
> {
>  int count = 0;
>
>  iterate_over_threads (inf_has_multiple_thread_cb, &count);
>  return (count > 1);
> }

right, I was afraid of adding to many new functions for a 'simple' bug
fix, but it does look better.

> Space before `;' again.
> ...
> Missing spaces again.

I'll take an extra care with all these spaces, sorry :)


Thanks,


Kevin

------

2011-04-12 Kevin Pouget <kevin.pouget@st.com>

	PR threads/12628

	* linux-fork.c (checkpoint_command): Disallow checkpointing of
	processes with multiple threads.
	(inf_has_multiple_thread_cb): New function.
	(inf_has_multiple_threads): New function.

-------

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 7f654af..e733cde 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -616,6 +616,33 @@ 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 *has_multiple_threads = (int *) data;
+
+  if (current_inferior ()->pid == ptid_get_pid (tp->ptid))
+    (*has_multiple_threads)++;
+
+  /* Stop the iteration if multiple threads have been detected.  */
+  return *has_multiple_threads > 1;
+}
+
+/* Return true if the current inferior is multi-threaded.  */
+
+static int
+inf_has_multiple_threads (void)
+{
+  int count = 0;
+
+  iterate_over_threads (inf_has_multiple_thread_cb, &count);
+  return (count > 1);
+}
+
 static void
 checkpoint_command (char *args, int from_tty)
 {
@@ -628,6 +655,14 @@ checkpoint_command (char *args, int from_tty)
   pid_t retpid;
   struct cleanup *old_chain;

+  if (!target_has_execution)
+    error (_("The program is not being run."));
+
+  /* Ensure that the inferior is not multithreaded.  */
+  update_thread_list ();
+  if (inf_has_multiple_threads ())
+    error (_("checkpoint: can't checkpoint multiple threads."));
+
   /* Make the inferior fork, record its (and gdb's) state.  */

   if (lookup_minimal_symbol ("fork", NULL, NULL) != NULL)