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

[Pedro Alves <palves@redhat.com>]

On 01/12/2017 03:34 PM, Yao Qi wrote:
> 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).

Note that's a _final_ cleanup.  I.e., it only runs when GDB exits.
There's no need for gdb_disassembler_test to destroy the stream
with that.

And in my palves/cxx-eliminate-cleanups branch, where I've
C++-fied the whole ui_file hierarchy, the cleanup completely
disappears.

Thanks,
Pedro Alves