Re: [PATCH 3/8] Disassembly unit test: disassemble one instruction

Yao Qi
2017-01-12T15:35:07Z

On 17-01-12 15:15:34, Pedro Alves wrote:
> On 01/10/2017 12:26 PM, Yao Qi wrote:
> > + class gdb_disassembler_test : public gdb_disassembler
> > + {
> > + public:
> > +
> > +#ifndef DISASSEMBLER_TEST_VERBOSE
> > + explicit gdb_disassembler_test (struct gdbarch *gdbarch,
> > + const gdb_byte *insn)
> > + : gdb_disassembler (gdbarch, ui_file_new (),
> > + gdb_disassembler_test::read_memory),
> > + m_insn (insn)
> > + {
> > + }
> > +
> > + ~gdb_disassembler_test ()
> > + {
> > + ui_file_delete ((struct ui_file *) m_di.stream);
>
>
> Hmm, looks like you've made m_di be "protected" for
> these uses.
>
> But we have the public stream() method already,
> so I think could be:
>
> ~gdb_disassembler_test ()
> {
> ui_file_delete (stream ());
> }
>
> You could then make m_di private again.
>
> But you shouldn't really need to create a new stream for
> testing. We have other places that want to print to a
> a null stream. We can factor out out the null_stream creation
> from gdb_insn_length into a new function:
>
> struct ui_file *
> null_stream ()
> {
> static struct ui_file *stream = NULL;
>
> if (stream == NULL)
> {
> stream = ui_file_new ();
> make_final_cleanup (do_ui_file_delete, stream);
> }
> return stream;
> }
>
> and then use it wherever necessary.
>

I did write code that way, but I changed it because the destroy of
stream is done in cleanup. I want the test case depends on other
part of GDB as less as possible. I hope this test case can be run
even without cleanup stuff. It is sort of RAII (stream is regarded
as resource).

--
Yao (é½å°§)