Re: [PATCH] Add control variable for debug output to linux threads

[Kevin Buettner <kevinb at cygnus dot com>]

On Apr 26,  4:13pm, Michael Snyder wrote:

> This patch replaces your DEBUG macro with a runtime variable that is
> settable by a user command.  At some recent check-in, you left the 
> DEBUG macro turned on, and users have been asking about the extra
> messages.
> 
> 2001-04-26  Michael Snyder  <msnyder@redhat.com>
> 
> 	* thread-db.c (_initialize_thread_db): Add set/show command
> 	"debug-linux-threads" for debugging output.
> 	* lin-lwp.c (various): Use global "debug_linux_threads to 
> 	turn on extra debugging output.

Michael,

I have problems with your patch since it does a bit more than
advertised.

First, the following portion reverts a change from Mark Kettenis:

> *************** check_event (int pid)
> *** 697,707 ****
>   	error ("Thread creation event doesn't match breakpoint.");
>   #endif
>   
> !       /* We may already know about this thread, for instance when the
> !          user has issued the `info threads' command before the SIGTRAP
> !          for hitting the thread creation breakpoint was reported.  */
> !       if (! in_thread_list (pid))
> ! 	attach_thread (pid, msg.th_p, &ti, 1);
>         return;
>   
>       case TD_DEATH:
> --- 701,710 ----
>   	error ("Thread creation event doesn't match breakpoint.");
>   #endif
>   
> !       if (in_thread_list (pid))
> ! 	error ("Spurious thread creation event.");
> ! 
> !       attach_thread (pid, msg.th_p, &ti, 1);
>         return;
>   
>       case TD_DEATH:

Mark's ChangeLog entry for the patch you reverted is as follows:

	* thread-db.c (check_event): Don't report an error if we encounter
	a thread creation event for a thread that's already in the thread
	list, since that may legitemately happen.  Instead only call
	attach_thread if it's not already in the thread list.

If you really did intend to revert his patch, you ought to provide
some justification for it along with a ChangeLog entry.

Next...

> *************** _initialize_thread_db (void)
> *** 1023,1029 ****
>         add_target (&thread_db_ops);
>   
>         /* Add ourselves to objfile event chain.  */
> !       target_new_objfile_chain = target_new_objfile_hook;
>         target_new_objfile_hook = thread_db_new_objfile;
>       }
>   }
> --- 1026,1037 ----
>         add_target (&thread_db_ops);
>   
>         /* Add ourselves to objfile event chain.  */
> !       new_objfile_event_chain = target_new_objfile_hook;
>         target_new_objfile_hook = thread_db_new_objfile;
>       }

Could you explain this (and related) changes?  I don't really see
a problem with it, but it'd be nice if there were a ChangeLog entry
to go along with it.

Kevin