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

Tom Tromey
2013-03-12T18:30:51Z

>>>>> "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