Re: [PATCH 0/3] Use selftest.c in GDBserver

[Pedro Alves <palves@redhat.com>]

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