This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/5] Save trace into CTF format
- From: Tom Tromey <tromey at redhat dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: <gdb-patches at sourceware dot org>
- Date: Thu, 07 Mar 2013 14:29:29 -0700
- Subject: Re: [PATCH 2/5] Save trace into CTF format
- References: <1361931459-3953-1-git-send-email-yao@codesourcery.com> <1361931459-3953-3-git-send-email-yao@codesourcery.com> <512F38CE.5030802@codesourcery.com> <87621avsam.fsf@fleche.redhat.com> <51332381.6010101@codesourcery.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