Re: [PATCH v3] Pass thread_info pointer to various inferior control functions

[Tom Tromey <tom@tromey.com>]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> I noticed that some functions in infcmd and infrun call each other and
Simon> all call inferior_thread, while they could just get the thread_info
Simon> pointer from their caller.  That means less calls to inferior_thread, so
Simon> less reliance on global state, since inferior_thread reads
Simon> inferior_ptid.

...

Simon> -set_step_frame (void)
Simon> +set_step_frame (thread_info *tp)
Simon>  {
Simon>    frame_info *frame = get_current_frame ();

I like the idea of passing parameters rather than relying on global
state.  However, in its current form, I think this patch may lull the
reader into a false sense of security.

That is, it makes it seems like these functions operate on a thread that
you pass in.  However, they don't actually.  For example, in this one, I
think get_current_frame must be relying on the global inferior_ptid.
So, this would change the function from something that obviously has to
be relying on globals to something that un-obviously is.

I don't know whether that's a reason to reject it or not.
It just seemed like maybe a future source of bugs.

I do welcome changes to reduce our reliance on globals.
Perhaps there's an argument that we can only achieve this incrementally
like this.  Though, the follow-on question there is whether this is
something that will actually happen.

Let me know what you think of this.

thanks,
Tom