From 45664f164a0a46612d06e7a91b0b0454f670f8db Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Thu, 22 Sep 2022 18:57:27 +0100 Subject: [PATCH] gdb/remote: remove some manual reference count handling While working on some other target_ops reference count related code, I spotted that in remote.c we do some manual reference count handling, i.e. we call target_ops::incref and decref_target (which wraps target_ops::decref). I think it would be better to make use of gdb::ref_ptr to automate the reference count management. So, this commit updates scoped_mark_target_starting in two ways, first, I use gdb::ref_ptr to handle the reference counts. Then, instead of using the scoped_mark_target_starting constructor and destructor to set and reset the starting_up flag, I now use a scoped_restore_tmpl object to set and restore the flag. The above changes mean that the scoped_mark_target_starting destructor can be completely removed, and the constructor body is now empty. I've also fixed a typo in the class comment. The only change in behaviour after this commit is that previously we didn't care what the value of starting_up was, we just set it to true in the constructor and false in the destructor. Now, I assert that the flag is initially false, then set the flag true when the scoped_mark_target_starting is created. As the starting_up flag is initialized to false then, for the assert to fire, we would need to recursively enter remote_target::start_remote_1, which I don't think is something we should be doing, so I think the new assert is an improvement. --- gdb/remote.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 53c4f19c5a4..614ee069a31 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -4702,7 +4702,7 @@ remote_target::process_initial_stop_replies (int from_tty) } } -/* Mark a remote_target as marking (by setting the starting_up flag within +/* Mark a remote_target as starting (by setting the starting_up flag within its remote_state) for the lifetime of this object. The reference count on the remote target is temporarily incremented, to prevent the target being deleted under our feet. */ @@ -4712,26 +4712,32 @@ struct scoped_mark_target_starting /* Constructor, TARGET is the target to be marked as starting, its reference count will be incremented. */ scoped_mark_target_starting (remote_target *target) - : m_remote_target (target) - { - m_remote_target->incref (); - remote_state *rs = m_remote_target->get_remote_state (); - rs->starting_up = true; - } + : m_remote_target (remote_target_ref::new_reference (target)), + m_restore_starting_up (set_starting_up_flag (target)) + { /* Nothing. */ } + +private: - /* Destructor, mark the target being worked on as no longer starting, and - decrement the reference count. */ - ~scoped_mark_target_starting () + /* Helper function, set the starting_up flag on TARGET and return an + object which, when it goes out of scope, will restore the previous + value of the starting_up flag. */ + static scoped_restore_tmpl + set_starting_up_flag (remote_target *target) { - remote_state *rs = m_remote_target->get_remote_state (); - rs->starting_up = false; - decref_target (m_remote_target); + remote_state *rs = target->get_remote_state (); + gdb_assert (!rs->starting_up); + return make_scoped_restore (&rs->starting_up, true); } -private: + /* A gdb::ref_ptr pointer to a remote_target. */ + using remote_target_ref = gdb::ref_ptr; + + /* A reference to the target on which we are operating. */ + remote_target_ref m_remote_target; - /* The target on which we are operating. */ - remote_target *m_remote_target; + /* An object which restores the previous value of the starting_up flag + when it goes out of scope. */ + scoped_restore_tmpl m_restore_starting_up; }; /* Helper for remote_target::start_remote, start the remote connection and -- 2.30.2