Re: [PATCH v3 01/15] Refactor 'tsave'

[Tom Tromey <tromey@redhat.com>]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> +struct tfile_trace_file_writer
Yao>  {
[...]
Yao> +  struct cleanup *cleanup;
Yao> +};

This makes it seem as though the writer has a cleanup chain separate
from the global chain.  But, this isn't the case.

This says to me that this field is a trap waiting to be sprung, say by
some future code rearrangement not taking this into account.

So, I think it would be better to have an explicit "dtor" method that is
called to clean up.

Yao> +  writer->ops->start (writer, filename);
Yao> +
Yao> +  writer->ops->write_header (writer);
Yao> +
Yao> +  /* Write descriptive info.  */
Yao> +
Yao> +  /* Write out the size of a register block.  */
Yao> +  writer->ops->write_regblock_type (writer, trace_regblock_size);
Yao> +
Yao> +  /* Write out status of the tracing run (aka "tstatus" info).  */
Yao> +  writer->ops->write_status (writer, ts);

It is strange to see multiple virtual calls in a row like this.

Is there some reason not to collapse them into a single call?

Tom