RE: [PING] [RFC] Thread debug support for NetBSD 5

["Paul Koning" <Paul_Koning@Dell.com>]

> > +static int pending_sigs;
> 
> I'm not sure whether this global can get stale between
> debug sessions or not, but it looked like it.  Say, if you
> kill a process while you have pending sigs, the next
> debug session will trip on it being != 0? 

Probably true, I'll fix that.

> It also points
> out that you should probably do something to the pending
> signals when you go about detaching from a process, so
> they don't get lost.

No, this variable is for reporting signals to gdb, so on a detach it
can/should be cleared.
 
> > +  if (catch_syscall_enabled () > 0)
> > +    request = PT_SYSCALL;
> > +  else
> > +    request = PT_CONTINUE;
> 
> I think this will be dead code, since you don't
> support inserting catch syscalls.

I copied that code out of inf-ptrace.c which is the common target code
for targets that use ptrace.  NetBSD was using that before. 
 
> > +  /* An address of (PTRACE_TYPE_ARG3)1 tells ptrace to continue
from
> > +     where it was.  If GDB wanted it to start some other way, we
> have
> > +     already written a new program counter value to the child.  */
> > +  errno = 0;
> 
> If this clearing of errno is needed, then it should move to just
> before the `ptrace' calls.  You have several function calls between
> this and the `ptrace' calls (at least when debugging is enable),
> and any of those could clobber `errno'.  (That's the reason
> for `save_errno' in your patch somewhere else, BTW.)

Right, I missed that when I added the debug messages.  Thanks.
 
> > +      /* If nothing found in the no wait case, report that.  */
> > +      if (options == WNOHANG && pid == 0)
> > +	return pid_to_ptid (-1);
> 
> Use minus_one_ptid, here and everywhere else.
> 
> 
> > +static char *
> > +nbsd_thread_pid_to_str (struct target_ops *ops, ptid_t ptid)
> > +{
> > +  if (TIDGET (ptid) == 0)
> > +    {
> > +      struct target_ops *beneath = find_target_beneath (ops);
> > +
> > +      return beneath->to_pid_to_str (beneath, ptid);
> > +    }
> > +  return xstrprintf (_("Thread %ld"), TIDGET (ptid));
> 
> This leaks.  Nothing ever releases the return of
> target_pid_to_str calls; that's why all implementations
> return a pointer to a static buffer.

Oops.  I copied that from dec-thread.c.
 
	paul