[PATCH] improved error message

[PATCH] improved error message

[Michael Snyder]

There are like five places in thread-db.c that all give the same
error message.  This just adds the function name, so you have a
chance of figuring out where the error occurred.

2002-12-13  Michael Snyder  <msnyder@to-limbo.toronto.redhat.com>

	* thread-db.c (thread_from_lwp): Uniquify error msg.
	(lwp_from_thread): Ditto.
	(check_event): Ditto.
	(find_new_threads_callback): Ditto.
	(thread_db_pid_to_str): Ditto.

Index: thread-db.c
===================================================================
RCS file: /cvs/src/src/gdb/thread-db.c,v
retrieving revision 1.25
diff -p -r1.25 thread-db.c
*** thread-db.c	23 Nov 2002 01:34:43 -0000	1.25
--- thread-db.c	13 Dec 2002 23:20:49 -0000
*************** thread_from_lwp (ptid_t ptid)
*** 250,256 ****
  
    err = td_thr_get_info_p (&th, &ti);
    if (err != TD_OK)
!     error ("Cannot get thread info: %s", thread_db_err_str (err));
  
    return BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
  }
--- 250,257 ----
  
    err = td_thr_get_info_p (&th, &ti);
    if (err != TD_OK)
!     error ("thread_from_lwp: cannot get thread info: %s", 
! 	   thread_db_err_str (err));
  
    return BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
  }
*************** lwp_from_thread (ptid_t ptid)
*** 272,278 ****
  
    err = td_thr_get_info_p (&th, &ti);
    if (err != TD_OK)
!     error ("Cannot get thread info: %s", thread_db_err_str (err));
  
    return BUILD_LWP (ti.ti_lid, GET_PID (ptid));
  }
--- 273,280 ----
  
    err = td_thr_get_info_p (&th, &ti);
    if (err != TD_OK)
!     error ("lwp_from_thread: cannot get thread info: %s", 
! 	   thread_db_err_str (err));
  
    return BUILD_LWP (ti.ti_lid, GET_PID (ptid));
  }
*************** check_event (ptid_t ptid)
*** 685,691 ****
  
    err = td_thr_get_info_p (msg.th_p, &ti);
    if (err != TD_OK)
!     error ("Cannot get thread info: %s", thread_db_err_str (err));
  
    ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
  
--- 687,694 ----
  
    err = td_thr_get_info_p (msg.th_p, &ti);
    if (err != TD_OK)
!     error ("check_event: cannot get thread info: %s", 
! 	   thread_db_err_str (err));
  
    ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
  
*************** find_new_threads_callback (const td_thrh
*** 950,956 ****
  
    err = td_thr_get_info_p (th_p, &ti);
    if (err != TD_OK)
!     error ("Cannot get thread info: %s", thread_db_err_str (err));
  
    if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
      return 0;			/* A zombie -- ignore.  */
--- 953,960 ----
  
    err = td_thr_get_info_p (th_p, &ti);
    if (err != TD_OK)
!     error ("find_new_threads_callback: cannot get thread info: %s", 
! 	   thread_db_err_str (err));
  
    if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
      return 0;			/* A zombie -- ignore.  */
*************** thread_db_pid_to_str (ptid_t ptid)
*** 993,999 ****
  
        err = td_thr_get_info_p (&th, &ti);
        if (err != TD_OK)
! 	error ("Cannot get thread info for thread %ld: %s",
  	       (long) GET_THREAD (ptid), thread_db_err_str (err));
  
        if (ti.ti_state == TD_THR_ACTIVE && ti.ti_lid != 0)
--- 997,1003 ----
  
        err = td_thr_get_info_p (&th, &ti);
        if (err != TD_OK)
! 	error ("thread_db_pid_to_str: cannot get thread info for %ld: %s",
  	       (long) GET_THREAD (ptid), thread_db_err_str (err));
  
        if (ti.ti_state == TD_THR_ACTIVE && ti.ti_lid != 0)

Re: [PATCH] improved error message

[Andrew Cagney]

> There are like five places in thread-db.c that all give the same
> error message.  This just adds the function name, so you have a
> chance of figuring out where the error occurred.
> 
> 2002-12-13  Michael Snyder  <msnyder@to-limbo.toronto.redhat.com>
> 
> 	* thread-db.c (thread_from_lwp): Uniquify error msg.
> 	(lwp_from_thread): Ditto.
> 	(check_event): Ditto.
> 	(find_new_threads_callback): Ditto.
> 	(thread_db_pid_to_str): Ditto.

Er, this looks wrong.  A normal user visible error message should not 
include references to internal function names.  Can you either reword 
the messages or, perhaphs, add a debug mode that displys this additional 
information?

Andrew


> Index: thread-db.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/thread-db.c,v
> retrieving revision 1.25
> diff -p -r1.25 thread-db.c
> *** thread-db.c	23 Nov 2002 01:34:43 -0000	1.25
> --- thread-db.c	13 Dec 2002 23:20:49 -0000
> *************** thread_from_lwp (ptid_t ptid)
> *** 250,256 ****
>   
>     err = td_thr_get_info_p (&th, &ti);
>     if (err != TD_OK)
> !     error ("Cannot get thread info: %s", thread_db_err_str (err));
>   
>     return BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
>   }
> --- 250,257 ----
>   
>     err = td_thr_get_info_p (&th, &ti);
>     if (err != TD_OK)
> !     error ("thread_from_lwp: cannot get thread info: %s", 
> ! 	   thread_db_err_str (err));
>   
>     return BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
>   }
> *************** lwp_from_thread (ptid_t ptid)
> *** 272,278 ****
>   
>     err = td_thr_get_info_p (&th, &ti);
>     if (err != TD_OK)
> !     error ("Cannot get thread info: %s", thread_db_err_str (err));
>   
>     return BUILD_LWP (ti.ti_lid, GET_PID (ptid));
>   }
> --- 273,280 ----
>   
>     err = td_thr_get_info_p (&th, &ti);
>     if (err != TD_OK)
> !     error ("lwp_from_thread: cannot get thread info: %s", 
> ! 	   thread_db_err_str (err));
>   
>     return BUILD_LWP (ti.ti_lid, GET_PID (ptid));
>   }
> *************** check_event (ptid_t ptid)
> *** 685,691 ****
>   
>     err = td_thr_get_info_p (msg.th_p, &ti);
>     if (err != TD_OK)
> !     error ("Cannot get thread info: %s", thread_db_err_str (err));
>   
>     ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
>   
> --- 687,694 ----
>   
>     err = td_thr_get_info_p (msg.th_p, &ti);
>     if (err != TD_OK)
> !     error ("check_event: cannot get thread info: %s", 
> ! 	   thread_db_err_str (err));
>   
>     ptid = BUILD_THREAD (ti.ti_tid, GET_PID (ptid));
>   
> *************** find_new_threads_callback (const td_thrh
> *** 950,956 ****
>   
>     err = td_thr_get_info_p (th_p, &ti);
>     if (err != TD_OK)
> !     error ("Cannot get thread info: %s", thread_db_err_str (err));
>   
>     if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
>       return 0;			/* A zombie -- ignore.  */
> --- 953,960 ----
>   
>     err = td_thr_get_info_p (th_p, &ti);
>     if (err != TD_OK)
> !     error ("find_new_threads_callback: cannot get thread info: %s", 
> ! 	   thread_db_err_str (err));
>   
>     if (ti.ti_state == TD_THR_UNKNOWN || ti.ti_state == TD_THR_ZOMBIE)
>       return 0;			/* A zombie -- ignore.  */
> *************** thread_db_pid_to_str (ptid_t ptid)
> *** 993,999 ****
>   
>         err = td_thr_get_info_p (&th, &ti);
>         if (err != TD_OK)
> ! 	error ("Cannot get thread info for thread %ld: %s",
>   	       (long) GET_THREAD (ptid), thread_db_err_str (err));
>   
>         if (ti.ti_state == TD_THR_ACTIVE && ti.ti_lid != 0)
> --- 997,1003 ----
>   
>         err = td_thr_get_info_p (&th, &ti);
>         if (err != TD_OK)
> ! 	error ("thread_db_pid_to_str: cannot get thread info for %ld: %s",
>   	       (long) GET_THREAD (ptid), thread_db_err_str (err));
>   
>         if (ti.ti_state == TD_THR_ACTIVE && ti.ti_lid != 0)
> 

Re: [PATCH] improved error message

[Daniel Jacobowitz]

On Mon, Dec 16, 2002 at 11:25:32AM -0500, Andrew Cagney wrote:
> >There are like five places in thread-db.c that all give the same
> >error message.  This just adds the function name, so you have a
> >chance of figuring out where the error occurred.
> >
> >2002-12-13  Michael Snyder  <msnyder@to-limbo.toronto.redhat.com>
> >
> >	* thread-db.c (thread_from_lwp): Uniquify error msg.
> >	(lwp_from_thread): Ditto.
> >	(check_event): Ditto.
> >	(find_new_threads_callback): Ditto.
> >	(thread_db_pid_to_str): Ditto.
> 
> Er, this looks wrong.  A normal user visible error message should not 
> include references to internal function names.  Can you either reword 
> the messages or, perhaphs, add a debug mode that displys this additional 
> information?

These are essentially internal error messages - none of them should
ever happen.  They do now, of course.

IMVHO I like Michael's change; now when people report these error
messages I'll know automatically which one they are.  They tend to be
only spontaneously reproducible.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] improved error message

[Andrew Cagney]

> On Mon, Dec 16, 2002 at 11:25:32AM -0500, Andrew Cagney wrote:
> 
>> >There are like five places in thread-db.c that all give the same
>> >error message.  This just adds the function name, so you have a
>> >chance of figuring out where the error occurred.
>> >
>> >2002-12-13  Michael Snyder  <msnyder@to-limbo.toronto.redhat.com>
>> >
>> >	* thread-db.c (thread_from_lwp): Uniquify error msg.
>> >	(lwp_from_thread): Ditto.
>> >	(check_event): Ditto.
>> >	(find_new_threads_callback): Ditto.
>> >	(thread_db_pid_to_str): Ditto.
> 
>> 
>> Er, this looks wrong.  A normal user visible error message should not 
>> include references to internal function names.  Can you either reword 
>> the messages or, perhaphs, add a debug mode that displys this additional 
>> information?
> 
> 
> These are essentially internal error messages - none of them should
> ever happen.  They do now, of course.
> 
> IMVHO I like Michael's change; now when people report these error
> messages I'll know automatically which one they are.  They tend to be
> only spontaneously reproducible.

If it is a internal error, call internal_error().

Andrew

Re: [PATCH] improved error message

[Daniel Jacobowitz]

On Mon, Dec 16, 2002 at 11:46:48AM -0500, Andrew Cagney wrote:
> >On Mon, Dec 16, 2002 at 11:25:32AM -0500, Andrew Cagney wrote:
> >
> >>>There are like five places in thread-db.c that all give the same
> >>>error message.  This just adds the function name, so you have a
> >>>chance of figuring out where the error occurred.
> >>>
> >>>2002-12-13  Michael Snyder  <msnyder@to-limbo.toronto.redhat.com>
> >>>
> >>>	* thread-db.c (thread_from_lwp): Uniquify error msg.
> >>>	(lwp_from_thread): Ditto.
> >>>	(check_event): Ditto.
> >>>	(find_new_threads_callback): Ditto.
> >>>	(thread_db_pid_to_str): Ditto.
> >
> >>
> >>Er, this looks wrong.  A normal user visible error message should not 
> >>include references to internal function names.  Can you either reword 
> >>the messages or, perhaphs, add a debug mode that displys this additional 
> >>information?
> >
> >
> >These are essentially internal error messages - none of them should
> >ever happen.  They do now, of course.
> >
> >IMVHO I like Michael's change; now when people report these error
> >messages I'll know automatically which one they are.  They tend to be
> >only spontaneously reproducible.
> 
> If it is a internal error, call internal_error().

From a practicality viewpoint, I refuse to do that when we know of
multiple situations which trigger these problems.  I still get them
reported to me every couple of weeks.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH] improved error message

[Andrew Cagney]

>> >These are essentially internal error messages - none of them should
>> >ever happen.  They do now, of course.
>> >
>> >IMVHO I like Michael's change; now when people report these error
>> >messages I'll know automatically which one they are.  They tend to be
>> >only spontaneously reproducible.
> 
>> 
>> If it is a internal error, call internal_error().
> 
> 
>>From a practicality viewpoint, I refuse to do that when we know of
> multiple situations which trigger these problems.  I still get them
> reported to me every couple of weeks.

Sorry I don't see the problem.  If it is an internal error then it 
should be reported as such.  That way the user can do things like 
provide us with a core dump.

Andrew

Re: [PATCH] improved error message

[Michal Ludvig]

Andrew Cagney wrote:
>> There are like five places in thread-db.c that all give the same
>> error message.  This just adds the function name, so you have a
>> chance of figuring out where the error occurred.
>>
>> 2002-12-13  Michael Snyder  <msnyder@to-limbo.toronto.redhat.com>
>>
>>     * thread-db.c (thread_from_lwp): Uniquify error msg.
>>     (lwp_from_thread): Ditto.
>>     (check_event): Ditto.
>>     (find_new_threads_callback): Ditto.
>>     (thread_db_pid_to_str): Ditto.
> 
> 
> Er, this looks wrong.  A normal user visible error message should not 
> include references to internal function names.  Can you either reword 
> the messages or, perhaphs, add a debug mode that displys this additional 
> information?

Why not? Having function names in error messages helps a lot in getting 
useful bugreports from users.

Michal Ludvig
-- 
* SuSE CR, s.r.o     * mludvig@suse.cz
* (+420) 296.545.373 * http://www.suse.cz