[rfa] tweak to defaulted_query logic to fix gdb.mi test errors
Commit Message
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
Comments
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?
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.
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
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\""
@@ -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);