This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 0/3] Use selftest.c in GDBserver
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Fri, 11 Aug 2017 18:11:08 +0100
- Subject: Re: [PATCH 0/3] Use selftest.c in GDBserver
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=palves at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 95594400F1
- References: <1502465408-24668-1-git-send-email-yao.qi@linaro.org>
On 08/11/2017 04:30 PM, Yao Qi wrote:
> This patch series is split from "[PATCH 00/26 v3] Make GDB builtin
> target descriptions more flexible"
> https://sourceware.org/ml/gdb-patches/2017-07/msg00076.html, because,
> IMO, adding self/unit tests in GDBserver doesn't related much to my
> target description patches. My target description patches only use
> common/selftest.c to do some unit tests in GDBserver.
>
> I also want to include one patch to use self tests in GDBserver, but
> can't find a module in GDBserver to test.
Nice, I had skimmed version of the patches in the large series a couple
weeks back, and was planning on suggesting to move the gdb-specifics to
hooks implemented by the client. Which is exactly what you've done.
Moving to selftests namespace seems like a good idea to me too.
I guess you could have left QUIT in the gdb hook, though it's fine with me
to remove it.
I wonder whether "gdbserver --help" should really advertise --selftests,
given it's a maintenance command and essentially does nothing in
release mode.
Implementing gdb's version of "selftest::reset ()" in selftest-arch.c
directly feels a bit like an abstraction violation. The cleanest would I
guess be to add a new .c file with reset() that calls a new
selftest-arch.c:reset_arch(), but we can always do that later if we need to
reset other things.
Otherwise, LGTM.
Actually, I sent a couple comments to patch #3 now. But it's
all minor things.
Thanks,
Pedro Alves