Re: [RFC] Never silently discard internal errors

[Michael Snyder <Michael dot Snyder at palmsource dot com>]

On Mon, 2006-09-25 at 14:42 -0400, Daniel Jacobowitz wrote:
> Here's a problem that one of our customers turned up.  He runs GDB
> from inside Emacs, which means that its standard input is considered
> to be a pipe, which is not a terminal.  Accordingly query is suppressed.
> Whether this is right everywhere or not is a question for another day,
> but one particular thing we discovered was that GDB would mysteriously
> quit out from under him, without telling him what it was doing.
> 
> Eventually we tracked it down to an unrelated problem in the code he was
> testing for me.  Not too surprising - that's what all the gdb_assert's
> and such are for, after all :-)  But not only was it not asking permission
> before it bailed, but it wasn't even leaving a useful message.
> 
> I think the attached patch is reasonable.  It changes error handling
> such that if query is a no-op - for instance, from a script file,
> or from a pipe - we dump out a message to stdout anyway before quitting
> or dumping core.
> 
> It's easy to see this behavior for yourself.  Just put the line
> "maint internal-error" in a text file, and start gdb with -x filename.
> Watch it abort without saying goodbye.
> 
> Any thoughts on this patch?  Shall I commit it?

I think the concept is sound.  I absolutely hate when gdb terminates
unexpectedly and doesn't tell me why; anything that reduces the odds
of that happening is good.



> Daniel Jacobowitz
> CodeSourcery
> 
> 2006-09-25  Daniel Jacobowitz  <dan@codesourcery.com>
> 
> 	* utils.c (query_is_silent): New.
> 	(internal_vproblem, query, defaulted_query): Use it.
> 
> Index: utils.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/utils.c,v
> retrieving revision 1.169
> diff -u -p -r1.169 utils.c
> --- utils.c	21 Sep 2006 13:50:51 -0000	1.169
> +++ utils.c	25 Sep 2006 18:31:50 -0000
> @@ -81,6 +81,8 @@ void (*deprecated_error_begin_hook) (voi
>  
>  /* Prototypes for local functions */
>  
> +static int query_is_silent (void);
> +
>  static void vfprintf_maybe_filtered (struct ui_file *, const char *,
>  				     va_list, int) ATTR_FORMAT (printf, 2, 0);
>  
> @@ -750,6 +752,9 @@ further debugging may prove unreliable."
>           this lessens the likelhood of GDB going into an infinate
>           loop.  */
>        quit_p = query (_("%s\nQuit this debugging session? "), reason);
> +      if (query_is_silent ())
> +	fprintf_unfiltered (gdb_stdout,
> +			    "%s\nQuitting this debugging session.\n", reason);
>        break;
>      case AUTO_BOOLEAN_TRUE:
>        quit_p = 1;
> @@ -768,7 +773,9 @@ further debugging may prove unreliable."
>           `dropping' so that it is easier to see that something went
>           wrong in GDB.  */
>        dump_core_p = query (_("%s\nCreate a core file of GDB? "), reason);
> -      break;
> +      if (query_is_silent ())
> +	fprintf_unfiltered (gdb_stdout,
> +			    "%s\nCreating a core file.\n", reason);
>        break;
>      case AUTO_BOOLEAN_TRUE:
>        dump_core_p = 1;
> @@ -1128,6 +1135,24 @@ gdb_print_host_address (const void *addr
>    fprintf_filtered (stream, "0x%lx", (unsigned long) addr);
>  }
>  
> +/* Return whether query will not display anything.  If it won't, the
> +   caller may want to display an informative message that would otherwise
> +   have been part of the query prompt.  Also used to implement query
> +   and defaulted_query, to assure they stay consistent.  */
> +
> +static int
> +query_is_silent (void)
> +{
> +  /* We will automatically answer the query if input is not from the
> +     user directly (e.g. from a script file or a pipe), or if the user
> +     did not want prompts.  */
> +  if (!input_from_terminal_p () || !caution)
> +    return 1;
> +
> +  return 0;
> +}
> +
> +
>  /* Ask user a y-or-n question and return 1 iff answer is yes.
>     Takes three args which are given to printf to print the question.
>     The first, a control string, should end in "? ".
> @@ -1142,9 +1167,8 @@ query (const char *ctlstr, ...)
>    int ans2;
>    int retval;
>  
> -  /* Automatically answer "yes" if input is not from the user
> -     directly, or if the user did not want prompts.  */
> -  if (!input_from_terminal_p () || !caution)
> +  /* Automatically answer "yes" if this query should not prompt.  */
> +  if (query_is_silent ())
>      return 1;
>  
>    if (deprecated_query_hook)
> @@ -1246,9 +1270,9 @@ defaulted_query (const char *ctlstr, con
>        n_string = "[n]";
>      }
>  
> -  /* Automatically answer the default value if input is not from the user
> -     directly, or if the user did not want prompts.  */
> -  if (!input_from_terminal_p () || !caution)
> +  /* Automatically answer the default value if this query should not
> +     prompt.  */
> +  if (query_is_silent ())
>      return def_value;
>  
>    if (deprecated_query_hook)