From: Pedro Alves <palves@redhat.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: Simon Marchi <simon.marchi@polymtl.ca>, gdb-patches@sourceware.org
Subject: Re: [RFC] (windows) GDB/MI crash when using "-list-thread-groups --available"
Date: Sat, 02 Jun 2018 10:26:00 -0000 [thread overview]
Message-ID: <afe333ff-38ef-3c74-d011-2151d909b5d6@redhat.com> (raw)
In-Reply-To: <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
next prev parent reply other threads:[~2018-06-02 10:26 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-10 19:12 Joel Brobecker
2018-05-10 21:10 ` Simon Marchi
2018-05-11 17:14 ` Pedro Alves
2018-05-16 15:52 ` Simon Marchi
2018-05-16 16:44 ` Pedro Alves
2018-06-02 0:59 ` Joel Brobecker
2018-06-02 10:26 ` Pedro Alves [this message]
2018-06-04 20:11 ` Joel Brobecker
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=afe333ff-38ef-3c74-d011-2151d909b5d6@redhat.com \
--to=palves@redhat.com \
--cc=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=simon.marchi@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).