[patch, rfa] tweak to defaulted_query logic to fix gdb.mi test errors

[patch, rfa] tweak to defaulted_query logic to fix gdb.mi test errors

[Sandra Loosemore]

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

This patch moves the check for deprecated_query_hook before 
input-not-from-terminal check in defaulted_query, instead of after it. 
Presently, the only thing that uses deprecated_query_hook is the MI 
backend, which uses it to suppress the question entirely, so that the 
current behavior leads to the situation where nothing is printed when 
the input is a terminal but there is a message otherwise, which is 
somewhat odd.

The specific problem this patch solves is a testing issue with 
gdbserver.  mi_gdb_target_load says:

mi_gdb_test "kill" ".*" ""

and if the test harness is not running GDB from a terminal we get:

Expecting: ^(kill[
]+)?(.*[
]+[(]gdb[)]
[ ]*)
kill
&"kill\n"
~"Kill the program being debugged? (y or n) [answered Y; input not from 
terminal]\n"
ERROR: Got interactive prompt.

because it is tripping over the "\\(y or n\\) " pattern in mi_gdb_test. 
  Of course mi_gdb_test could be fixed to recognize the "input not from 
terminal" message as valid, but I don't think that's the intended MI 
behavior.  I think this is a bug in GDB, not the testsuite.

OK to commit?

-Sandra


[-- Attachment #2: gdb-query-fsf.log --]
[-- Type: text/x-log, Size: 165 bytes --]

2016-09-06  Sandra Loosemore  <sandra@codesourcery.com>

	gdb/
	* utils.c (defaulted_query): Swap order of deprecated_query_hook
	and input-not-from-terminal cases.

[-- Attachment #3: gdb-query-fsf.patch --]
[-- Type: text/x-patch, Size: 1217 bytes --]

diff --git a/gdb/utils.c b/gdb/utils.c
index 5188828..b1e08a6 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1292,6 +1292,16 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
   if (!confirm || server_command)
     return def_value;
 
+  if (deprecated_query_hook)
+    {
+      int res;
+
+      old_chain = make_cleanup_restore_target_terminal ();
+      res = deprecated_query_hook (ctlstr, args);
+      do_cleanups (old_chain);
+      return res;
+    }
+
   /* If input isn't coming from the user directly, just say what
      question we're asking, and then answer the default automatically.  This
      way, important error messages don't get lost when talking to GDB
@@ -1314,16 +1324,6 @@ defaulted_query (const char *ctlstr, const char defchar, va_list args)
       return def_value;
     }
 
-  if (deprecated_query_hook)
-    {
-      int res;
-
-      old_chain = make_cleanup_restore_target_terminal ();
-      res = deprecated_query_hook (ctlstr, args);
-      do_cleanups (old_chain);
-      return res;
-    }
-
   /* Format the question outside of the loop, to avoid reusing args.  */
   question = xstrvprintf (ctlstr, args);
   old_chain = make_cleanup (xfree, question);

Re: [patch, rfa] tweak to defaulted_query logic to fix gdb.mi test errors

[Luis Machado]

On 09/06/2016 01:56 PM, Sandra Loosemore wrote:
> This patch moves the check for deprecated_query_hook before
> input-not-from-terminal check in defaulted_query, instead of after it.
> Presently, the only thing that uses deprecated_query_hook is the MI
> backend, which uses it to suppress the question entirely, so that the
> current behavior leads to the situation where nothing is printed when
> the input is a terminal but there is a message otherwise, which is
> somewhat odd.
>
> The specific problem this patch solves is a testing issue with
> gdbserver.  mi_gdb_target_load says:
>
> mi_gdb_test "kill" ".*" ""
>
> and if the test harness is not running GDB from a terminal we get:
>
> Expecting: ^(kill[
> ]+)?(.*[
> ]+[(]gdb[)]
> [ ]*)
> kill
> &"kill\n"
> ~"Kill the program being debugged? (y or n) [answered Y; input not from
> terminal]\n"
> ERROR: Got interactive prompt.
>
> because it is tripping over the "\\(y or n\\) " pattern in mi_gdb_test.
>  Of course mi_gdb_test could be fixed to recognize the "input not from
> terminal" message as valid, but I don't think that's the intended MI
> behavior.  I think this is a bug in GDB, not the testsuite.
>
> OK to commit?
>
> -Sandra
>

This looks simple and reasonable enough for me. What do 
others/maintainers think?

Re: [patch, rfa] tweak to defaulted_query logic to fix gdb.mi test errors

[Yao Qi]

On Tue, Sep 6, 2016 at 2:56 PM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
> This patch moves the check for deprecated_query_hook before
> input-not-from-terminal check in defaulted_query, instead of after it.
> Presently, the only thing that uses deprecated_query_hook is the MI backend,
> which uses it to suppress the question entirely, so that the current
> behavior leads to the situation where nothing is printed when the input is a
> terminal but there is a message otherwise, which is somewhat odd.
>
> The specific problem this patch solves is a testing issue with gdbserver.
> mi_gdb_target_load says:
>
> mi_gdb_test "kill" ".*" ""
>
> and if the test harness is not running GDB from a terminal we get:
>

Can you "set interactive-mode on" in your test?  I read your patch several
times, I am not sure we need this patch.

-- 
Yao (齐尧)

Re: [patch, rfa] tweak to defaulted_query logic to fix gdb.mi test errors

[Sandra Loosemore]

On 10/18/2016 02:16 PM, Yao Qi wrote:
> On Tue, Sep 6, 2016 at 2:56 PM, Sandra Loosemore
> <sandra@codesourcery.com> wrote:
>> This patch moves the check for deprecated_query_hook before
>> input-not-from-terminal check in defaulted_query, instead of after it.
>> Presently, the only thing that uses deprecated_query_hook is the MI backend,
>> which uses it to suppress the question entirely, so that the current
>> behavior leads to the situation where nothing is printed when the input is a
>> terminal but there is a message otherwise, which is somewhat odd.
>>
>> The specific problem this patch solves is a testing issue with gdbserver.
>> mi_gdb_target_load says:
>>
>> mi_gdb_test "kill" ".*" ""
>>
>> and if the test harness is not running GDB from a terminal we get:
>>
>
> Can you "set interactive-mode on" in your test?  I read your patch several
> times, I am not sure we need this patch.
>

I'm sorry, but what test are you referring to?  There was no testcase 
included with the patch, and the bug affects a whole bunch of the 
existing gdb.mi testcases.

-Sandra

Re: [patch, rfa] tweak to defaulted_query logic to fix gdb.mi test errors

[Yao Qi]

On Tue, Oct 18, 2016 at 9:24 PM, Sandra Loosemore
<sandra@codesourcery.com> wrote:
>
> I'm sorry, but what test are you referring to?  There was no testcase
> included with the patch, and the bug affects a whole bunch of the existing
> gdb.mi testcases.
>

When you run GDB tests without terminal, always "set interactive-mode on".
You can do this below in your board file.

set GDBFLAGS "${GDBFLAGS} -ex \"set interactive-mode on\""

-- 
Yao (齐尧)