This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH/RFC] Refactor gdb.reverse/insn-reverse.c
On 01/25/2017 11:44 AM, Pedro Alves wrote:
On 01/25/2017 04:50 PM, Luis Machado wrote:
On 01/25/2017 10:10 AM, Yao Qi wrote:
On 17-01-16 13:10:54, Luis Machado wrote:
I've broken up the main file into other files with arch-specific bits
(insn-support-<arch>.c). The main file will hold the generic pieces
take care of calling the tests.
It is reasonable to me. Can we name arch-specific files as
Thanks for the review.
Would you reconsider this? I named it insn-support-<arch>.c because we
already know this is a reverse debugging test (from gdb.reverse) and we
are really testing instruction support. I'm fine either way though, and
just wanted to add a little bit more context in the name.
My 2c nit.
"support" doesn't sound ideal to me. By that logic,
every test in the testsuite is testing gdb's support of some
feature, so "support" is redundant?
When I see a file named "support", I think it's some base/foundation
(==support) framework. Is that the case here? I had understood
that the "base" is in insn-reverse.c instead? Or are the
insn-support-<arch>.c files meant to be shared between multiple
test cases, thus they'll be providing "support" for a multitude
of random tests? insn-reverse-<arch>.c at least gave a hint
that the files are all related to insn-reverse.c.
Also, if "reverse" is redundant in "insn-reverse-<arch>.c", then
it is also redundant in "insn-reverse.c" too, so you should be arguing
for renaming "insn-reverse.c|exp" -> "insn-support.c|exp" too, which
would preserve the similarity of the names between the driver
file + the arch files.
That is a reasonable assessment. insn-reverse.[c|exp] is redundant and
IMO would benefit from renaming too.
The support in "insn-support-<arch>.c means support for a set of
instructions for this particular subsystem of gdb, therefore why i went
with that name. Thinking about it further, instruction decoding support
is the basis/foundation of reverse debugging, without which things would
not work properly. But i may be overthinking. :-)
I didn't analyze it in depth though. It just seemed to me the naming was
slightly better with less redundancy.
But, like i said, i'm fine either way.