set record query <on|off>

set record query <on|off>

[Marc Khouzam]

Hi,

here is a patch to allow to enable/disable the queries
used in Record.  It is relevant to:

http://sourceware.org/ml/gdb/2009-09/msg00165.html

What do you think?

Sorry, I have to run, but I'll send the Changelog tonight.

Thanks

Marc

P.S. I didn't make the change in record_check_insn_num()
because I got the feeling that function may need review.

### Eclipse Workspace Patch 1.0
#P src
Index: gdb/record.c
===================================================================
RCS file: /cvs/src/src/gdb/record.c,v
retrieving revision 1.17
diff -u -r1.17 record.c
--- gdb/record.c        8 Sep 2009 00:50:42 -0000       1.17
+++ gdb/record.c        11 Sep 2009 14:51:48 -0000
@@ -94,6 +94,14 @@
 static int record_insn_max_num = DEFAULT_RECORD_INSN_MAX_NUM;
 static int record_insn_num = 0;

+/* 1 use queries.  0 don't use queries and perform action.
+   Note that when queries are not used, we do not fall back to
+   any specified query default; instead, we perform the action.
+   This is important for frontends that don't handle queries or
+   to allow actions to be performed even if 'set confirm off'
+   is used */
+static int record_enable_queries = 1;
+
 /* The target_ops of process record.  */
 static struct target_ops record_ops;

@@ -443,9 +451,9 @@
   /* 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?")))
+      if (record_enable_queries
+         && !nquery (_("Process record target already running, do you want to "
+                       "delete the old record log?")))
        return;
     }

@@ -961,19 +969,24 @@
          int n;

          /* Let user choose if he wants to write register or not.  */
-         if (regno < 0)
-           n =
-             nquery (_("Because GDB is in replay mode, changing the "
-                       "value of a register will make the execution "
-                       "log unusable from this point onward.  "
-                       "Change all registers?"));
+         if (record_enable_queries)
+           {
+             if (regno < 0)
+               n =
+                 nquery (_("Because GDB is in replay mode, changing the "
+                           "value of a register will make the execution "
+                           "log unusable from this point onward.  "
+                           "Change all registers?"));
+             else
+               n =
+                 nquery (_("Because GDB is in replay mode, changing the value "
+                           "of a register will make the execution log unusable "
+                           "from this point onward.  Change register %s?"),
+                         gdbarch_register_name (get_regcache_arch (regcache),
+                                                  regno));
+           }
          else
-           n =
-             nquery (_("Because GDB is in replay mode, changing the value "
-                       "of a register will make the execution log unusable "
-                       "from this point onward.  Change register %s?"),
-                     gdbarch_register_name (get_regcache_arch (regcache),
-                                              regno));
+           n = 1;

          if (!n)
            {
@@ -1019,10 +1032,11 @@
       if (RECORD_IS_REPLAY)
        {
          /* Let user choose if he wants to write memory or not.  */
-         if (!nquery (_("Because GDB is in replay mode, writing to memory "
-                        "will make the execution log unusable from this "
-                        "point onward.  Write memory at address %s?"),
-                      paddress (target_gdbarch, offset)))
+         if (record_enable_queries
+             && !nquery (_("Because GDB is in replay mode, writing to memory "
+                           "will make the execution log unusable from this "
+                           "point onward.  Write memory at address %s?"),
+                         paddress (target_gdbarch, offset)))
            error (_("Process record canceled the operation."));

          /* Destroy the record from here forward.  */
@@ -1163,9 +1177,11 @@
     {
       if (RECORD_IS_REPLAY)
        {
-         if (!from_tty || query (_("Delete the log from this point forward "
-                                   "and begin to record the running message "
-                                   "at current PC?")))
+         if (!from_tty
+             || !record_enable_queries
+             || query (_("Delete the log from this point forward "
+                         "and begin to record the running message "
+                         "at current PC?")))
            record_list_release_next ();
        }
       else
@@ -1183,8 +1199,10 @@
 {
   if (current_target.to_stratum == record_stratum)
     {
-      if (!record_list || !from_tty || query (_("Delete recorded log and "
-                                               "stop recording?")))
+      if (!record_list
+         || !from_tty
+         || !record_enable_queries
+         || query (_("Delete recorded log and stop recording?")))
        unpush_target (&record_ops);
     }
   else
@@ -1310,4 +1328,18 @@
   add_cmd ("insn-number", class_obscure, show_record_insn_number,
           _("Show the current number of instructions in the "
             "record/replay buffer."), &info_record_cmdlist);
+
+  /* Command to enable/disable the user of queries in Process Record */
+  add_setshow_boolean_cmd ("query", no_class,
+                          &record_enable_queries, _("\
+Set whether record/replay should enable its use of queries."), _("\
+Show whether record/replay should enable its use of queries."), _("\
+Default is ON.\n\
+When ON, record/replay will query the user on how to behave for certain \
+scenarios.\n\
+When OFF, record/replay will never query the user and will assume that \
+all record actions normally requiring a query, should be performed.\n\
+This option is valuable for frontends which cannot handle the use of queries."),
+                          NULL, NULL,
+                          &set_record_cmdlist, &show_record_cmdlist);
 }

Re: set record query <on|off>

[Tom Tromey]

>>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:

Marc> here is a patch to allow to enable/disable the queries
Marc> used in Record.  It is relevant to:
Marc> http://sourceware.org/ml/gdb/2009-09/msg00165.html
Marc> What do you think?

It seems like a strange approach to me.

I don't think I know the whole background here.  Is this to help users
who enter commands at a console?  Or MI?  Or CLI commands sent by
Eclipse, but not from a user console (that is, in response to some GUI
control)?

For MI, it seems like there should be arguments to the internal function
and then the MI commands should disable querying that way.  That is, you
don't need a new user-visible setting for this.

For the CLI-commands-sent-by-Eclipse case, it seems like you could use
the "server" prefix.

For the console case... it seems to me that queries are just a necessary
part of that.

Tom

RE: set record query <on|off>

[Marc Khouzam]

> -----Original Message-----
> From: Tom Tromey [mailto:tromey@redhat.com] 
> Sent: September-11-09 6:07 PM
> To: Marc Khouzam
> Cc: 'gdb-patches@sourceware.org'
> Subject: Re: set record query <on|off>
> 
> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
> 
> Marc> here is a patch to allow to enable/disable the queries
> Marc> used in Record.  It is relevant to:
> Marc> http://sourceware.org/ml/gdb/2009-09/msg00165.html
> Marc> What do you think?
> 
> It seems like a strange approach to me.

I was hoping for something more generic for queries, but in the few
email exchanges, things kept coming back to such a specific new 
Record command.

> I don't think I know the whole background here.  Is this to help users
> who enter commands at a console?  Or MI?  Or CLI commands sent by
> Eclipse, but not from a user console (that is, in response to some GUI
> control)?

Sorry, I should have summarized the issue in the mail.  Here goes.

When Eclipse is interacting with GDB, there is a single "channel of
communication" which is used for everything (MI, CLI when no MI equivalent,
and Eclipse console).  This channel triggers the GDB code paths
that are "!input_from_terminal_p ()" and this makes every query
be automatically answered with the default answer.

This was not a problem before because there was only a single
use of nquery() and it was never triggered by Eclipse
(it was for pending breakpoints).
With PRecord though, there are new uses of nquery(), and these, from Eclipse,
keep being answer 'N' automatically.

Only one case is actually a problem right now:
it is when changing memory (or registers) while replaying execution; in this
case there is an nquery(), and since it is automatically answered, I just
cannot set memory during an replay.  Being able to change memory during
replay is one of the cool features of PRecord, and that is why I'm trying
really hard to get this to work for 7.0.
BTW, to attempt to set memory, we use -var-assign so I cannot use the 
"server" prefix.

> For MI, it seems like there should be arguments to the 
> internal function
> and then the MI commands should disable querying that way.  
> That is, you
> don't need a new user-visible setting for this.

Disabling queries based on MI is something that was brought up.
The idea was to have queries check if the interpreter was MI and in
this case, answer 'Y' to any query.
I like this because it is more future-proof.
Is that what you are suggesting too?
 
> For the CLI-commands-sent-by-Eclipse case, it seems like you could use
> the "server" prefix.

This may be useful for other cases, but in the current case (-var-assign) 
it does not work.
 
> For the console case... it seems to me that queries are just 
> a necessary part of that.

It would be nicer to have queries in this case but not only does it
not work because we are in the case !input_from_terminal_p (), but having
a query would block the "communication channel" waiting for user input, 
which would somewhat lock-up the Eclipse GUI.

So, to summarize the summary, I need a way to get GDB to answer 'Y' for every
query, even nquery().  And right now, the only time this is needed is for PRecord.

Thanks for any more guidance.

Marc

Re: set record query <on|off>

[Hui Zhu]

If you don't mind.  I suggest you discussion "Eclipse handle query" in
a special thread.

Let's talk about my idea about this patch.  I think give a switch to
close the query in the prec is not bad.  Even if we don't use Eclipse,
we still need close query sometime.
But someone want this swich always "yes", someone want always "no".

So I suggest each query have a special "switch" to control it.
For example:
set record query off
set record changememory yes
set record changereg yes

If query is on, this yes or no choice yquery or nquery.  If query is
off, this yes or no can be the answer.

What do you think about it?


Thanks,
Hui


On Sat, Sep 12, 2009 at 09:08, Marc Khouzam <marc.khouzam@ericsson.com> wrote:
>> -----Original Message-----
>> From: Tom Tromey [mailto:tromey@redhat.com]
>> Sent: September-11-09 6:07 PM
>> To: Marc Khouzam
>> Cc: 'gdb-patches@sourceware.org'
>> Subject: Re: set record query <on|off>
>>
>> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
>>
>> Marc> here is a patch to allow to enable/disable the queries
>> Marc> used in Record.  It is relevant to:
>> Marc> http://sourceware.org/ml/gdb/2009-09/msg00165.html
>> Marc> What do you think?
>>
>> It seems like a strange approach to me.
>
> I was hoping for something more generic for queries, but in the few
> email exchanges, things kept coming back to such a specific new
> Record command.
>
>> I don't think I know the whole background here.  Is this to help users
>> who enter commands at a console?  Or MI?  Or CLI commands sent by
>> Eclipse, but not from a user console (that is, in response to some GUI
>> control)?
>
> Sorry, I should have summarized the issue in the mail.  Here goes.
>
> When Eclipse is interacting with GDB, there is a single "channel of
> communication" which is used for everything (MI, CLI when no MI equivalent,
> and Eclipse console).  This channel triggers the GDB code paths
> that are "!input_from_terminal_p ()" and this makes every query
> be automatically answered with the default answer.
>
> This was not a problem before because there was only a single
> use of nquery() and it was never triggered by Eclipse
> (it was for pending breakpoints).
> With PRecord though, there are new uses of nquery(), and these, from Eclipse,
> keep being answer 'N' automatically.
>
> Only one case is actually a problem right now:
> it is when changing memory (or registers) while replaying execution; in this
> case there is an nquery(), and since it is automatically answered, I just
> cannot set memory during an replay.  Being able to change memory during
> replay is one of the cool features of PRecord, and that is why I'm trying
> really hard to get this to work for 7.0.
> BTW, to attempt to set memory, we use -var-assign so I cannot use the
> "server" prefix.
>
>> For MI, it seems like there should be arguments to the
>> internal function
>> and then the MI commands should disable querying that way.
>> That is, you
>> don't need a new user-visible setting for this.
>
> Disabling queries based on MI is something that was brought up.
> The idea was to have queries check if the interpreter was MI and in
> this case, answer 'Y' to any query.
> I like this because it is more future-proof.
> Is that what you are suggesting too?
>
>> For the CLI-commands-sent-by-Eclipse case, it seems like you could use
>> the "server" prefix.
>
> This may be useful for other cases, but in the current case (-var-assign)
> it does not work.
>
>> For the console case... it seems to me that queries are just
>> a necessary part of that.
>
> It would be nicer to have queries in this case but not only does it
> not work because we are in the case !input_from_terminal_p (), but having
> a query would block the "communication channel" waiting for user input,
> which would somewhat lock-up the Eclipse GUI.
>
> So, to summarize the summary, I need a way to get GDB to answer 'Y' for every
> query, even nquery().  And right now, the only time this is needed is for PRecord.
>
> Thanks for any more guidance.
>
> Marc
>

RE: set record query <on|off>

[Marc Khouzam]

> -----Original Message-----
> From: Hui Zhu [mailto:teawater@gmail.com] 
> Sent: September-11-09 10:57 PM
> To: Marc Khouzam
> Cc: tromey@redhat.com; gdb-patches@sourceware.org; Michael Snyder
> Subject: Re: set record query <on|off>
> 
> If you don't mind.  I suggest you discussion "Eclipse handle query" in
> a special thread.

You are probably right that it may be a good way to proceed.
I'll try to come up with an other suggest and start a new thread also.
 
> Let's talk about my idea about this patch.  I think give a switch to
> close the query in the prec is not bad.  Even if we don't use Eclipse,
> we still need close query sometime.
> But someone want this swich always "yes", someone want always "no".
> 
> So I suggest each query have a special "switch" to control it.
> For example:
> set record query off
> set record changememory yes
> set record changereg yes
> 
> If query is on, this yes or no choice yquery or nquery.  If query is
> off, this yes or no can be the answer.
> 
> What do you think about it?

I see your point and it may be useful to some people.
On the other hand it may be going a bit far in giving the user
different configuration options.

In any case, if this approach is the one chosen, it would solve
my problem, so I'm not against it :-) but I would lean towards
something simpler.  But that is just me.

Thanks

Marc


> 
> 
> Thanks,
> Hui
> 
> 
> On Sat, Sep 12, 2009 at 09:08, Marc Khouzam 
> <marc.khouzam@ericsson.com> wrote:
> >> -----Original Message-----
> >> From: Tom Tromey [mailto:tromey@redhat.com]
> >> Sent: September-11-09 6:07 PM
> >> To: Marc Khouzam
> >> Cc: 'gdb-patches@sourceware.org'
> >> Subject: Re: set record query <on|off>
> >>
> >> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
> >>
> >> Marc> here is a patch to allow to enable/disable the queries
> >> Marc> used in Record.  It is relevant to:
> >> Marc> http://sourceware.org/ml/gdb/2009-09/msg00165.html
> >> Marc> What do you think?
> >>
> >> It seems like a strange approach to me.
> >
> > I was hoping for something more generic for queries, but in the few
> > email exchanges, things kept coming back to such a specific new
> > Record command.
> >
> >> I don't think I know the whole background here.  Is this 
> to help users
> >> who enter commands at a console?  Or MI?  Or CLI commands sent by
> >> Eclipse, but not from a user console (that is, in response 
> to some GUI
> >> control)?
> >
> > Sorry, I should have summarized the issue in the mail.  Here goes.
> >
> > When Eclipse is interacting with GDB, there is a single "channel of
> > communication" which is used for everything (MI, CLI when 
> no MI equivalent,
> > and Eclipse console).  This channel triggers the GDB code paths
> > that are "!input_from_terminal_p ()" and this makes every query
> > be automatically answered with the default answer.
> >
> > This was not a problem before because there was only a single
> > use of nquery() and it was never triggered by Eclipse
> > (it was for pending breakpoints).
> > With PRecord though, there are new uses of nquery(), and 
> these, from Eclipse,
> > keep being answer 'N' automatically.
> >
> > Only one case is actually a problem right now:
> > it is when changing memory (or registers) while replaying 
> execution; in this
> > case there is an nquery(), and since it is automatically 
> answered, I just
> > cannot set memory during an replay.  Being able to change 
> memory during
> > replay is one of the cool features of PRecord, and that is 
> why I'm trying
> > really hard to get this to work for 7.0.
> > BTW, to attempt to set memory, we use -var-assign so I 
> cannot use the
> > "server" prefix.
> >
> >> For MI, it seems like there should be arguments to the
> >> internal function
> >> and then the MI commands should disable querying that way.
> >> That is, you
> >> don't need a new user-visible setting for this.
> >
> > Disabling queries based on MI is something that was brought up.
> > The idea was to have queries check if the interpreter was MI and in
> > this case, answer 'Y' to any query.
> > I like this because it is more future-proof.
> > Is that what you are suggesting too?
> >
> >> For the CLI-commands-sent-by-Eclipse case, it seems like 
> you could use
> >> the "server" prefix.
> >
> > This may be useful for other cases, but in the current case 
> (-var-assign)
> > it does not work.
> >
> >> For the console case... it seems to me that queries are just
> >> a necessary part of that.
> >
> > It would be nicer to have queries in this case but not only does it
> > not work because we are in the case !input_from_terminal_p 
> (), but having
> > a query would block the "communication channel" waiting for 
> user input,
> > which would somewhat lock-up the Eclipse GUI.
> >
> > So, to summarize the summary, I need a way to get GDB to 
> answer 'Y' for every
> > query, even nquery().  And right now, the only time this is 
> needed is for PRecord.
> >
> > Thanks for any more guidance.
> >
> > Marc
> >
> 

Re: set record query <on|off>

[Hui Zhu]

On Mon, Sep 14, 2009 at 08:21, Marc Khouzam <marc.khouzam@ericsson.com> wrote:
>
>> -----Original Message-----
>> From: Hui Zhu [mailto:teawater@gmail.com]
>> Sent: September-11-09 10:57 PM
>> To: Marc Khouzam
>> Cc: tromey@redhat.com; gdb-patches@sourceware.org; Michael Snyder
>> Subject: Re: set record query <on|off>
>>
>> If you don't mind.  I suggest you discussion "Eclipse handle query" in
>> a special thread.
>
> You are probably right that it may be a good way to proceed.
> I'll try to come up with an other suggest and start a new thread also.
>
>> Let's talk about my idea about this patch.  I think give a switch to
>> close the query in the prec is not bad.  Even if we don't use Eclipse,
>> we still need close query sometime.
>> But someone want this swich always "yes", someone want always "no".
>>
>> So I suggest each query have a special "switch" to control it.
>> For example:
>> set record query off
>> set record changememory yes
>> set record changereg yes
>>
>> If query is on, this yes or no choice yquery or nquery.  If query is
>> off, this yes or no can be the answer.
>>
>> What do you think about it?
>
> I see your point and it may be useful to some people.
> On the other hand it may be going a bit far in giving the user
> different configuration options.
>
> In any case, if this approach is the one chosen, it would solve
> my problem, so I'm not against it :-) but I would lean towards
> something simpler.  But that is just me.
>
> Thanks
>
> Marc
>
>
>>
>>
>> Thanks,
>> Hui
>>
>>
>> On Sat, Sep 12, 2009 at 09:08, Marc Khouzam
>> <marc.khouzam@ericsson.com> wrote:
>> >> -----Original Message-----
>> >> From: Tom Tromey [mailto:tromey@redhat.com]
>> >> Sent: September-11-09 6:07 PM
>> >> To: Marc Khouzam
>> >> Cc: 'gdb-patches@sourceware.org'
>> >> Subject: Re: set record query <on|off>
>> >>
>> >> >>>>> "Marc" == Marc Khouzam <marc.khouzam@ericsson.com> writes:
>> >>
>> >> Marc> here is a patch to allow to enable/disable the queries
>> >> Marc> used in Record.  It is relevant to:
>> >> Marc> http://sourceware.org/ml/gdb/2009-09/msg00165.html
>> >> Marc> What do you think?
>> >>
>> >> It seems like a strange approach to me.
>> >
>> > I was hoping for something more generic for queries, but in the few
>> > email exchanges, things kept coming back to such a specific new
>> > Record command.
>> >
>> >> I don't think I know the whole background here.  Is this
>> to help users
>> >> who enter commands at a console?  Or MI?  Or CLI commands sent by
>> >> Eclipse, but not from a user console (that is, in response
>> to some GUI
>> >> control)?
>> >
>> > Sorry, I should have summarized the issue in the mail.  Here goes.
>> >
>> > When Eclipse is interacting with GDB, there is a single "channel of
>> > communication" which is used for everything (MI, CLI when
>> no MI equivalent,
>> > and Eclipse console).  This channel triggers the GDB code paths
>> > that are "!input_from_terminal_p ()" and this makes every query
>> > be automatically answered with the default answer.
>> >
>> > This was not a problem before because there was only a single
>> > use of nquery() and it was never triggered by Eclipse
>> > (it was for pending breakpoints).
>> > With PRecord though, there are new uses of nquery(), and
>> these, from Eclipse,
>> > keep being answer 'N' automatically.
>> >
>> > Only one case is actually a problem right now:
>> > it is when changing memory (or registers) while replaying
>> execution; in this
>> > case there is an nquery(), and since it is automatically
>> answered, I just
>> > cannot set memory during an replay.  Being able to change
>> memory during
>> > replay is one of the cool features of PRecord, and that is
>> why I'm trying
>> > really hard to get this to work for 7.0.
>> > BTW, to attempt to set memory, we use -var-assign so I
>> cannot use the
>> > "server" prefix.
>> >
>> >> For MI, it seems like there should be arguments to the
>> >> internal function
>> >> and then the MI commands should disable querying that way.
>> >> That is, you
>> >> don't need a new user-visible setting for this.
>> >
>> > Disabling queries based on MI is something that was brought up.
>> > The idea was to have queries check if the interpreter was MI and in
>> > this case, answer 'Y' to any query.
>> > I like this because it is more future-proof.
>> > Is that what you are suggesting too?
>> >
>> >> For the CLI-commands-sent-by-Eclipse case, it seems like
>> you could use
>> >> the "server" prefix.
>> >
>> > This may be useful for other cases, but in the current case
>> (-var-assign)
>> > it does not work.
>> >
>> >> For the console case... it seems to me that queries are just
>> >> a necessary part of that.
>> >
>> > It would be nicer to have queries in this case but not only does it
>> > not work because we are in the case !input_from_terminal_p
>> (), but having
>> > a query would block the "communication channel" waiting for
>> user input,
>> > which would somewhat lock-up the Eclipse GUI.
>> >
>> > So, to summarize the summary, I need a way to get GDB to
>> answer 'Y' for every
>> > query, even nquery().  And right now, the only time this is
>> needed is for PRecord.
>> >
>> > Thanks for any more guidance.
>> >
>> > Marc
>> >
>>

Hi Marc,

If you really cannot handle query with Eclipse, I think this patch is
OK with me.  Because I really want prec in 7.0 can work well with
Eclipse.  I will add switch for each query.

BTW, I suggest don't add "set record query <on|off>" to doc.  Because
I don't want simple customer use it because we have switch for each
query.

Thanks,
Hui

RE: set record query <on|off>

[Marc Khouzam]

 

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org 
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Hui Zhu
> Sent: Monday, September 14, 2009 1:45 AM
> To: Marc Khouzam
> Cc: tromey@redhat.com; gdb-patches@sourceware.org; Michael Snyder
> Subject: Re: set record query <on|off>
> 
...
> 
> Hi Marc,
> 
> If you really cannot handle query with Eclipse, I think this patch is
> OK with me.  Because I really want prec in 7.0 can work well with
> Eclipse.  I will add switch for each query.

If you are going to add a switch for each query, that will also fix
my problem.  So that is ok for me.  I also really want prec to work
well with Eclipse :-)

So, I'll wait for your patches instead.

Thanks,
Marc
 

Re: set record query <on|off>

[Hui Zhu]

>
> If you are going to add a switch for each query, that will also fix
> my problem.  So that is ok for me.  I also really want prec to work
> well with Eclipse :-)

Sorry for my poor english.  My mean is prec need "set record query
off" and "set record changememory yes".

We can handle this issue step by step if you really cannot handle
query with Eclipse.
1.  "set record query off"
2. "set record changememory yes"

Thanks,
Hui