Re: [PATCH v2] PR threads/20743: Don't attempt to suspend or resume exited threads.

[Luis Machado <lgustavo@codesourcery.com>]

On 04/14/2017 05:40 PM, John Baldwin wrote:
> Hmm, the raw code looks fine.  I know that my MUA (kmail) messes up formatting
> of code as it displays tabs as 4 characters instead of 8?  Here's the raw
> code with tabs expanded to spaces:

Ah, that could very well be it.

>
>       ALL_NON_EXITED_THREADS (tp)
>         {
>           if (ptid_get_pid (tp->ptid) != ptid_get_pid (ptid))
>             continue;
>
>           if (ptid_get_lwp (tp->ptid) == ptid_get_lwp (ptid))
>             request = PT_RESUME;
>           else
>             request = PT_SUSPEND;
>
>           if (ptrace (request, ptid_get_lwp (tp->ptid), NULL, 0) == -1)
>             perror_with_name (("ptrace"));
>         }
>

The indentation here looks fine indeed.

>> A question i have is why did we have to remove the original functions.
>> Couldn't we have checked the non-exited-ness of the threads inside the
>> callback?
>
> That was what the V1 patch did, but you and Pedro requested it use
> ALL_NON_EXITED_THREADS instead, hence version 2.
>

Sorry, i swapped out the context of v1.

>> Another bit... Since we're changing this code, might as well improve the
>> perror message so it is more meaningful?
>
> I could perhaps do a followup to include the ptrace op in the various
> perror's in this file (all of them use this, as do the various BSD
> nat.c files used for register fetch/store).
>

That sounds like a good idea and could be postponed to a more convenient 
time.

>> Otherwise i have no further comments. I assume you ran gdb's testsuite
>> against this change and verified the results are sane?
>
> There were no regressions at least.  With the stock tree there are
> several unexpected failures already which I will get to at some point.
>

Great. I have no further comments.