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

[Tom Tromey <tromey@redhat.com>]

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