diff mbox series

Pass unique_ptr to add_thread_with_info

Message ID 20230705224247.536013-1-tom@tromey.com
State New
Headers
Series Pass unique_ptr to add_thread_with_info |

Commit Message

Tom Tromey July 5, 2023, 10:42 p.m. UTC 
  This changes add_thread_with_info to accept a unique_ptr, making it
clear that it takes ownership of the passed-in pointer.

I can't test the AIX or Darwin changes, but I think they are
relatively obvious.
---
 gdb/aix-thread.c      | 2 +-
 gdb/darwin-nat.c      | 3 ++-
 gdb/gdbthread.h       | 7 +++++--
 gdb/linux-thread-db.c | 3 ++-
 gdb/thread.c          | 4 ++--
 5 files changed, 12 insertions(+), 7 deletions(-)

Comments

Simon Marchi July 6, 2023, 2:29 a.m. UTC | #1 
I think that's a step in the right direction.  The callers could be
improved to use unique_ptr local variables and std::move it to the
add_thread_with_info call, but this patch is also fine as-is.

> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index 7135515bf45..8b5ec8abf01 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -222,6 +222,9 @@ struct private_thread_info
>    virtual ~private_thread_info () = 0;
>  };
>  
> +/* Unique pointer wrapper for private_thread_info.  */
> +typedef std::unique_ptr<private_thread_info> private_thread_info_up;

I've been meaning to ask for a while, is there a reason you keep using
typedef instead of the more C++y using?

Simon
Tom Tromey July 6, 2023, 3:16 p.m. UTC | #2 
>>>>> Simon Marchi <simark@simark.ca> writes:

>> +/* Unique pointer wrapper for private_thread_info.  */
>> +typedef std::unique_ptr<private_thread_info> private_thread_info_up;

> I've been meaning to ask for a while, is there a reason you keep using
> typedef instead of the more C++y using?

Inertia.  I'll change it.

Tom
Tom Tromey July 6, 2023, 3:17 p.m. UTC | #3 
Forgot to mention...

> I think that's a step in the right direction.  The callers could be
> improved to use unique_ptr local variables and std::move it to the
> add_thread_with_info call, but this patch is also fine as-is.

I considered this but since I can't build all the targets, it seemed a
bit riskier.

Tom
Tom Tromey Aug. 10, 2023, 4:50 p.m. UTC | #4 
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> This changes add_thread_with_info to accept a unique_ptr, making it
Tom> clear that it takes ownership of the passed-in pointer.

Tom> I can't test the AIX or Darwin changes, but I think they are
Tom> relatively obvious.

I'd forgotten about this.
I'm checking in the updated version now.

Tom
diff mbox series

Patch

diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index fbe80d656c2..4508ca9dca0 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -909,7 +909,7 @@  sync_threadlists (pid_t pid)
 
 	  thread = add_thread_with_info (proc_target,
 					 ptid_t (pid, 0, pbuf[pi].pthid),
-					 priv);
+					 private_thread_info_up (priv));
 
 	  pi++;
 	}
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index 4cf2d9f387a..588e9e2a345 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -351,7 +351,8 @@  darwin_nat_target::check_new_threads (inferior *inf)
 	  pti->msg_state = DARWIN_RUNNING;
 
 	  /* Add the new thread.  */
-	  add_thread_with_info (this, ptid_t (inf->pid, 0, new_id), pti);
+	  add_thread_with_info (this, ptid_t (inf->pid, 0, new_id),
+				private_thread_info_up (pti));
 	  new_thread_vec.push_back (pti);
 	  new_ix++;
 	  continue;
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 7135515bf45..8b5ec8abf01 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -222,6 +222,9 @@  struct private_thread_info
   virtual ~private_thread_info () = 0;
 };
 
+/* Unique pointer wrapper for private_thread_info.  */
+typedef std::unique_ptr<private_thread_info> private_thread_info_up;
+
 /* Threads are intrusively refcounted objects.  Being the
    user-selected thread is normally considered an implicit strong
    reference and is thus not accounted in the refcount, unlike
@@ -522,7 +525,7 @@  class thread_info : public refcounted_object,
   struct frame_id initiating_frame = null_frame_id;
 
   /* Private data used by the target vector implementation.  */
-  std::unique_ptr<private_thread_info> priv;
+  private_thread_info_up priv;
 
   /* Branch trace information for this thread.  */
   struct btrace_thread_info btrace {};
@@ -616,7 +619,7 @@  extern struct thread_info *add_thread_silent (process_stratum_target *targ,
 /* Same as add_thread, and sets the private info.  */
 extern struct thread_info *add_thread_with_info (process_stratum_target *targ,
 						 ptid_t ptid,
-						 private_thread_info *);
+						 private_thread_info_up);
 
 /* Delete thread THREAD and notify of thread exit.  If the thread is
    currently not deletable, don't actually delete it but still tag it
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 71a81aa0cb9..7d9fd57da0c 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -1366,7 +1366,8 @@  record_thread (struct thread_db_info *info,
      thread with this PTID, but it's marked exited, then the kernel
      reused the tid of an old thread.  */
   if (tp == NULL || tp->state == THREAD_EXITED)
-    tp = add_thread_with_info (info->process_target, ptid, priv);
+    tp = add_thread_with_info (info->process_target, ptid,
+			       private_thread_info_up (priv));
   else
     tp->priv.reset (priv);
 
diff --git a/gdb/thread.c b/gdb/thread.c
index 7f7f035b5ab..63ed87e9aa6 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -306,11 +306,11 @@  add_thread_silent (process_stratum_target *targ, ptid_t ptid)
 
 struct thread_info *
 add_thread_with_info (process_stratum_target *targ, ptid_t ptid,
-		      private_thread_info *priv)
+		      private_thread_info_up priv)
 {
   thread_info *result = add_thread_silent (targ, ptid);
 
-  result->priv.reset (priv);
+  result->priv = std::move (priv);
 
   if (print_thread_events)
     gdb_printf (_("[New %s]\n"), target_pid_to_str (ptid).c_str ());