Re: [PATCH 2/5] Save trace into CTF format

Tom Tromey
2013-03-07T21:29:41Z

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

Yao> +static void
Yao> +ctf_save_fwrite (FILE *fd, const gdb_byte *buf, size_t size)
Yao> +{
Yao> + if (fwrite (buf, size, 1, fd) != 1)
Yao> + error (_("Unable to write file for saving trace data (%s)"),
Yao> + safe_strerror (errno));
Yao> +}

Why not merge this function with its only caller?

Yao> +static void
Yao> +ctf_save_fwrite_format (FILE *fd, const char *format, ...)
Yao> +{
Yao> + va_list args;
Yao> +
Yao> + va_start (args, format);
Yao> + vfprintf (fd, format, args);

This seems like it needs the same error-checking as ctf_save_fwrite;
plus maybe (?) the content_size update as well.

Yao> +static void
Yao> +ctf_write_frame_v_block (struct trace_file_writer *self,
Yao> + int num, LONGEST val)
Yao> +{
Yao> + struct ctf_trace_file_writer *writer
Yao> + = (struct ctf_trace_file_writer *) self;
Yao> + uint32_t id = CTF_EVENT_ID_TSV;
Yao> +
Yao> + /* Event Id. */
Yao> + ctf_save_align_write (&writer->tcs, (gdb_byte *) &id, 4, 4);
Yao> +
Yao> + /* val. */
Yao> + ctf_save_align_write (&writer->tcs, (gdb_byte *) &val, 8, 8);

It seems safer to use a uint64_t intermediary here.

Yao> + /* num. */
Yao> + ctf_save_align_write (&writer->tcs, (gdb_byte *) &num, 4, 4);

And a uint32_t here.

Yao> +/* Save the trace data to dir DIRENAME of ctf format. */

Typo, "DIRNAME".

Yao> +void
Yao> +trace_save_ctf (const char *dirname, int target_does_save)
Yao> +{
Yao> + struct trace_file_writer *writer;
Yao> + struct cleanup *back_to;
Yao> +
Yao> + writer = ctf_trace_file_writer_new ();
Yao> + trace_save (dirname, writer, target_does_save);
Yao> + back_to = make_cleanup (trace_file_writer_xfree, writer);
Yao> + do_cleanups (back_to);

I think the make_cleanup and trace_save lines have to be exchanged.
Otherwise this doesn't make much sense.

Tom