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

[Yao Qi <qiyaoltc at gmail dot com>]

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 (齐尧)