Re: [PATCH] gdb/amd-dbgapi-target: Use inf param in detach

[Pedro Alves <pedro@palves.net>]

On 2023-07-14 19:54, Lancelot SIX wrote:
> On 14/07/2023 19:35, Pedro Alves wrote:

>> IMO this is a case of the target method's inferior * parameter having
>> been added too soon -- it would only make sense to have it if nothing in
>> the body of the target method implementations is relying on inf being
>> current_inferior on entry.  But that is not the case, plenty of target_ops::detach
>> code has that assumption.  The presence of an explicit inferior pointer should mean
>> that detach target method implementations that call code that depends
>> on the inferior being the current inferior, should be using a
>> scoped_restore_current_thread/inferior before calling such global-state-assuming
>> code.  But, they don't do that, instead, we have this mixed situation.  IMO, it would
>> be better to remove the parameter to avoid confusion and stick to the
>> (if explicit param, then switch global state to it if you need it) rule.
>>
>> Anyhow, your patch doesn't make it worse, so it's fine with me.
>>
> 
> I can also go the other way around and always use `current_inferior ()` instead of the `inf` parameter in this detach implementation.
> 
> What bugged me here is the inconsistency from one line to the next.
> 

Please go ahead and merge your patch.  I quickly looked at what it would like to remove
the parameter throughout, and I think we'd just end up doing:

  inferior *inf = current_inferior ();

at the top of the function, so using "inf" in the remove_breakpoints_inf call anyhow, as
we have multiple references to the current inferior.

Pedro Alves