Re: RFC: fix problems with errors during quitting (killed.exp)

["J. Johnston" <jjohnstn@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]



Andrew Cagney wrote:
>>> My new strategy is to intercept error() calls during quitting.  
>>> Basically,
>>> a simple get/set function is set up to denote when we the user has 
>>> confirmed
>>> a quit.
>>
>>
>>
>> My immediate concern is, does killed.exp leave a stopped binary around?
>>
>> Other than that I like it.
> 
> 
> I feel ill.
> 

I'm sorry to hear that.

> The first bug is in quit_force, so modifying throw_exeption to work 
> around it is a hack.  quit_force needs to catch any errors thrown by the 
> target vector.
> 
> Looking at the function's body.  I think the first part:
> 
>>   /* An optional expression may be used to cause gdb to terminate with 
>> the      value of that expression. */
>>   if (args)
>>     {
>>       struct value *val = parse_and_eval (args);
>>
>>       exit_code = (int) value_as_long (val);
>>     }
> 
> 
> should still be allowed to error out.  Otherwize something like:
> 
>     (gdb) exit bogus operand
> 
> will quit gdb denying the user of an oportunity to enter the correct 
> command.  The target calls and (I guess) the write_history should be 
> captured:
> 
>>   if (! ptid_equal (inferior_ptid, null_ptid) && target_has_execution)
>>     {
>>       if (attach_flag)
>>         target_detach (args, from_tty);
>>       else
>>         target_kill ();
>>     }
>>
>>   /* UDI wants this, to kill the TIP.  */
>>   target_close (1);
>>
>>   /* Save the history information if it is appropriate to do so.  */
>>   if (write_history_p && history_filename)
>>     write_history (history_filename);
> 
> 
> Also putting the do_final_cleanups inside the safety net would also 
> probably be for the best.
> 
>>   do_final_cleanups (ALL_CLEANUPS);     /* Do any final cleanups 
>> before exiting 
> 
> 

Ok, your idea is much better.  I have reworked the patch to use catch_errors().  I prepended the
message "Quitting: " so users will know that even though there is an error message
appended, it is still quitting.  I could optionally have put "Error quitting: "
but I thought the former was a better choice.

New patch attached.

2003-09-11  Jeff Johnston  <jjohnstn@redhat.com>

	* top.c (quit_target): New static helper function.
	(quit_force): Moved code to quit_target().  Call quit_target()
	via catch_errors() to catch errors during quit.

Ok to commit?

-- Jeff J.

> 
> 
> 
> The second less urgent bug is in the target code.  The target code 
> should catch the error and then force itself to be popped (then 
> rethrowing the error I guess).  That would have ensured that forward 
> progress was made and that second quit attemt worked.
> 
> Andrew
> 
> 

[-- Attachment #2: killed-patch2 --]
[-- Type: text/plain, Size: 1834 bytes --]

Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.75
diff -u -r1.75 top.c
--- top.c	16 Aug 2003 18:38:46 -0000	1.75
+++ top.c	11 Sep 2003 19:42:03 -0000
@@ -1439,28 +1439,25 @@
   return 1;
 }
 
-/* Quit without asking for confirmation.  */
+/* Helper routine for quit_force that requires error handling.  */
 
-void
-quit_force (char *args, int from_tty)
+struct qt_args
 {
-  int exit_code = 0;
-
-  /* An optional expression may be used to cause gdb to terminate with the 
-     value of that expression. */
-  if (args)
-    {
-      struct value *val = parse_and_eval (args);
+  char *args;
+  int from_tty;
+};
 
-      exit_code = (int) value_as_long (val);
-    }
+static int
+quit_target (void *arg)
+{
+  struct qt_args *qt = (struct qt_args *)arg;
 
   if (! ptid_equal (inferior_ptid, null_ptid) && target_has_execution)
     {
       if (attach_flag)
-	target_detach (args, from_tty);
+        target_detach (qt->args, qt->from_tty);
       else
-	target_kill ();
+        target_kill ();
     }
 
   /* UDI wants this, to kill the TIP.  */
@@ -1471,6 +1468,29 @@
     write_history (history_filename);
 
   do_final_cleanups (ALL_CLEANUPS);	/* Do any final cleanups before exiting */
+
+  return 0;
+}
+
+/* Quit without asking for confirmation.  */
+
+void
+quit_force (char *args, int from_tty)
+{
+  int exit_code = 0;
+
+  /* An optional expression may be used to cause gdb to terminate with the 
+     value of that expression. */
+  if (args)
+    {
+      struct value *val = parse_and_eval (args);
+
+      exit_code = (int) value_as_long (val);
+    }
+
+  /* We want to handle any quit errors and exit regardless.  */
+  catch_errors (quit_target, args,
+	        "Quitting: ", RETURN_MASK_ALL);
 
   exit (exit_code);
 }