Re: Testing of reverse debug commands

[Hui Zhu <teawater@gmail.com>]

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

On Mon, Jul 13, 2009 at 03:25, Marc Khouzam<marc.khouzam@ericsson.com> wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:pedro@codesourcery.com]
>> Sent: July-12-09 3:08 PM
>> To: gdb@sourceware.org
>> Cc: Marc Khouzam; Michael Snyder; Jan Kratochvil; Joel
>> Brobecker; Hui Zhu
>> Subject: Re: Testing of reverse debug commands
>>
>> On Sunday 12 July 2009 19:49:58, Marc Khouzam wrote:
>>
>> > From a frontend perspective, the current 'record' command
>> is not very
>> > good.
>>
>> > First, there is no MI equivalent, although that is not a big deal.
>> > But since it does not report error, the frontend must always assumes
>> > that
>> > the command worked.
>>
>> Are you sure that is the case with "record"?  I see `error' calls
>> in record.c:record_open.  If there are some missing, let's add them.
>
> As usual you are right. :-)
>
> (gdb) record
> &"record\n"
> &"Process record target can't debug inferior in asynchronous mode
> (target-async).\n"
> ^error,msg="Process record target can't debug inferior in asynchronous
> mode (target-async)."
> (gdb)
>
> I had made the assumption that 'record stop' was the same as 'record'.
>
>> > Below you can see that using 'record stop' directly will
>> give a ^done
>> > instead of an ^error when it fails (although there is an
>> error message
>> > but we don't parse those in Eclipse).  Also, using -interpreter-exec
>> > is even worse as even the error message is gone.
>>
>> Note that "record stop" is really a different command, although
>> it shares a common "record" prefix.
>>
>> >
>> > (gdb)
>> > record stop
>> > &"record stop\n"
>> > ~"Process record is not started.\n"
>> > ^done
>> > (gdb)
>>
>> > So, I think some improvement would be nice for frontends.
>>
>> So, is this really an error?  Hui seems to have thought
>> it wasn't.  Hui?  If it is, then it's just a matter of
>> changing the corresponding printf_unfiltered calls in
>> record.c to `error' calls (look for the "Process record
>> is..." string).
>> Then MI will get an ^error,msg="foo", instead of a ~"foo" + ^done.
>
> That would be more consistent for a frontend.  The frontend
> can then decide if this should be reported as an error or simply
> accepted.  But that is not such a big deal anymore, now that
> you pointed out 'record' itself does report an error.
>

I think the record's query and something is make a lot of troubles.
I make a patch for it.  Please help me with it.

Thanks,
Hui

2009-07-13  Hui Zhu  <teawater@gmail.com>
	* record.c (record_open): Change "query" to "error".
	(cmd_record_stop): Change "query" to "printf_unfiltered".
---
 record.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

--- a/record.c
+++ b/record.c
@@ -436,12 +436,8 @@ record_open (char *name, int from_tty)

   /* Check if record target is already running.  */
   if (current_target.to_stratum == record_stratum)
-    {
-      if (!nquery
-	  (_("Process record target already running, do you want to delete "
-	     "the old record log?")))
-	return;
-    }
+    error (_("Process record target already running.  Use \"record stop\" to "
+	     "stop record target first."));

   /*Reset the beneath function pointers.  */
   record_beneath_to_resume = NULL;
@@ -1157,9 +1153,9 @@ cmd_record_stop (char *args, int from_tt
 {
   if (current_target.to_stratum == record_stratum)
     {
-      if (!record_list || !from_tty || query (_("Delete recorded log and "
-	                                        "stop recording?")))
-	unpush_target (&record_ops);
+      unpush_target (&record_ops);
+      printf_unfiltered (_("Process record is stoped and all execution "
+                           "log is deleted.\n"));
     }
   else
     printf_unfiltered (_("Process record is not started.\n"));

[-- Attachment #2: prec_remove_query.txt --]
[-- Type: text/plain, Size: 1166 bytes --]

---
 record.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

--- a/record.c
+++ b/record.c
@@ -436,12 +436,8 @@ record_open (char *name, int from_tty)
 
   /* Check if record target is already running.  */
   if (current_target.to_stratum == record_stratum)
-    {
-      if (!nquery
-	  (_("Process record target already running, do you want to delete "
-	     "the old record log?")))
-	return;
-    }
+    error (_("Process record target already running.  Use \"record stop\" to "
+	     "stop record target first."));
 
   /*Reset the beneath function pointers.  */
   record_beneath_to_resume = NULL;
@@ -1157,9 +1153,9 @@ cmd_record_stop (char *args, int from_tt
 {
   if (current_target.to_stratum == record_stratum)
     {
-      if (!record_list || !from_tty || query (_("Delete recorded log and "
-	                                        "stop recording?")))
-	unpush_target (&record_ops);
+      unpush_target (&record_ops);
+      printf_unfiltered (_("Process record is stoped and all execution "
+                           "log is deleted.\n"));
     }
   else
     printf_unfiltered (_("Process record is not started.\n"));