This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFC] (windows) GDB/MI crash when using "-list-thread-groups --available"
- From: Pedro Alves <palves at redhat dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: Simon Marchi <simon dot marchi at polymtl dot ca>, gdb-patches at sourceware dot org
- Date: Sat, 2 Jun 2018 11:26:17 +0100
- Subject: Re: [RFC] (windows) GDB/MI crash when using "-list-thread-groups --available"
- References: <1525978704-70543-1-git-send-email-brobecker@adacore.com> <eaa0d8a4e091e97392897d5960df0149@polymtl.ca> <ba03aa7a-1474-2329-da05-038520f0eae2@redhat.com> <20180602005941.f4l5nudlsl3xez4v@adacore.com>
On 06/02/2018 01:59 AM, Joel Brobecker wrote:
> Thanks for the pointer. I've taken the liberty of updating our
> testcase cookbook to reference it as well, so we know where to
> look next time we hit that issue.
Good idea, thanks.
>
> Attached is an updated version which follows the same principle.
>
> gdb/ChangeLog:
>
> * windows-nat.c (windows_nat_target::xfer_partial): Return
> TARGET_XFER_E_IO if we need to delegate to the target beneath
> but BENEATH is NULL.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.mi/mi-list-thread-groups-no-inferior.exp: New testcase.
>
> Test verified on x86_64-linux to confirm that the large amount of
> output is properly handled.
>
> OK to push?
It looks good to me. A few comments on minor details follow.
How about naming the testcase list-thread-groups-no-inferior.exp
(no "mi-") so that it sits side-by-side with
list-thread-groups-available.exp?
We're missing an intro comment in the .exp file mentioning what the
testcase is about, otherwise it's not clear why we have two separate
testcases for seemingly the same thing.
A couple microscopic nits more:
> +gdb_test_multiple $test $test {
> + -re ".*\}" {
Leading ".*" is not necessary, it's implied.
> +mi_gdb_test "-data-evaluate-expression 1" \
> + ".*\\^done,value=\"1\"" \
> + "check GDB is still alive"
> +
"check" and "test" in test messages is one of my pet peeves.
OK, I'm really exaggerating. :-) The point is that all
tests are checking something, making it redundant.
"GDB is still alive" or even "GDB is alive" says the same.
Thanks,
Pedro Alves