A couple of comments about the nptl patch (minor)

A couple of comments about the nptl patch (minor)

[Joel Brobecker]

Hello,

I'm reading Jeff's patch regarding support for NPTL, and have a couple
of comments/questions... It's really minor...

> static int tkill_failed;

This could be moved inside kill_lwp. Something like the following.
I would also explicitely set it to zero but this is maybe already
guaranteed by the language?

> #ifdef HAVE_TKILL_SYSCALL
>   if (!tkill_failed)
>     {
        static int tkill_failed = 0;
>       int ret = syscall (__NR_tkill, lwpid, signo);
>       if (errno != ENOSYS)
>         return ret;
>       errno = 0;
>       tkill_failed = 1;
>     }
> #endif

In stop_callback, the gdb_assert for ret to be zero after the call to
kill (or kill_lwp) has been removed. I suppose it is valid for syscall
to return nonzero values ? In that case, is it still necessary to keep
this local variable?

> /* Send a SIGSTOP to LP.  */
> 
> static int
> stop_callback (struct lwp_info *lp, void *data)
> {
>   if (!lp->stopped && !lp->signalled)
>     {
>       int ret;
[...]
>       ret = kill_lwp (GET_LWP (lp->ptid), SIGSTOP);
[...]
>     }

-- 
Joel

Re: A couple of comments about the nptl patch (minor)

[J. Johnston]

Joel Brobecker wrote:
> Hello,
> 
> I'm reading Jeff's patch regarding support for NPTL, and have a couple
> of comments/questions... It's really minor...
> 
> 
>>static int tkill_failed;
> 
> 
> This could be moved inside kill_lwp. Something like the following.
> I would also explicitely set it to zero but this is maybe already
> guaranteed by the language?
> 

That would move the declaration after the first reference (the if statement).
C defaults static and global variables to 0 if they have no initial value specified.
So, yes, it is guaranteed by the language.

> 
>>#ifdef HAVE_TKILL_SYSCALL
>>  if (!tkill_failed)
>>    {
> 
>         static int tkill_failed = 0;
> 
>>      int ret = syscall (__NR_tkill, lwpid, signo);
>>      if (errno != ENOSYS)
>>        return ret;
>>      errno = 0;
>>      tkill_failed = 1;
>>    }
>>#endif
> 


> 
> In stop_callback, the gdb_assert for ret to be zero after the call to
> kill (or kill_lwp) has been removed. I suppose it is valid for syscall
> to return nonzero values ? In that case, is it still necessary to keep
> this local variable?
> 

I removed the gdb_assert because in nptl, the lwp can exit without you getting
notified (you don't get an exit event like you did under linuxthreads).  Daniel J.
wants to change this, but it already a behavior of RH9.  The ret is kept for the
debugging output when "set debug lin-lwp 1" is specified.

> 
>>/* Send a SIGSTOP to LP.  */
>>
>>static int
>>stop_callback (struct lwp_info *lp, void *data)
>>{
>>  if (!lp->stopped && !lp->signalled)
>>    {
>>      int ret;
> 
> [...]
> 
>>      ret = kill_lwp (GET_LWP (lp->ptid), SIGSTOP);
> 
> [...]
> 
>>    }
> 
>