Re: [PATCH 10/31] Thread options & clone events (Linux GDBserver)

Pedro Alves
2023-11-13T14:09:18Z

On 2023-06-06 15:12, Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:

>> @@ -3527,7 +3538,8 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
>>
>> /* Break the unreported fork relationship chain. */
>> if (event_child->waitstatus.kind () == TARGET_WAITKIND_FORKED
>> - || event_child->waitstatus.kind () == TARGET_WAITKIND_VFORKED)
>> + || event_child->waitstatus.kind () == TARGET_WAITKIND_VFORKED
>> + || event_child->waitstatus.kind () == TARGET_WAITKIND_THREAD_CLONED)
>
> Can we not use:
>
> if (is_new_child_status (event_child->waitstatus.kind ()))
>
> here?
>

We can! Done.

> I almost asked here: do we really need to check for THREAD_CLONED here,
> given the body of the `if` only changes the `fork_relative` field, which
> surely is only related to forks, right?
>
> ... but then I dug deeper and spotted that the field also used for clone
> events now, so: I wonder if we should rename `fork_relative` to
> something better, maybe `fork_clone_relative`, or maybe go generic with
> something like `related_lwp`?
>
> It just seemed a little confusing (to me).
>
> But otherwise, this patch looks good.
>

I've renamed the field to just "relative". That made me spot a few more places where
the comments could be adjusted to not refer only to "fork".

> Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks!