Re: [PATCH 1/5] Refactor 'tsave'

Yao Qi
2013-03-01T08:58:03Z

[-- Attachment #1: Type: text/plain, Size: 4075 bytes --]

Hi Pedro,
thanks for the quick review. The updated patch address most of your
comments except some I'd like to discuss with you below,

On 03/01/2013 12:33 AM, Pedro Alves wrote:
> Ok, I think I got it.
>
> I think an important missing clue here is that "raw" in
> "write_raw_data" and target_get_raw_trace_data actually means
> frames in "tfile" format, right? "ops->write_raw_data" being
> NULL means that the writer can't handle parsing the "tfile"
> frames itself.
>
> "raw" here is almost meaningless to the reader -- surely "raw" in
> the ctf context could mean raw ctf "something".
> Maybe "raw" should really be s/raw/tfile/ or s/raw/raw_tfile/ ?
>

"raw" here means the data in the trace buffers, and "raw data" has few
to do with the file format. It is only about how data are stored in
trace buffers. See the comments on write_raw_data,

/* Write the raw data of trace buffer. The content is in BUF and
length is LEN. */
void (*write_raw_data) (struct trace_file_writer *self,
gdb_byte *buf, LONGEST len);

I'd like to differentiate the "raw data" stored in trace buffers and the
data finally saved into files.

TFILE is a format that composed by ascii definition part and trace
frames dumped from the raw data directly. There could be another trace
file format FOO that stores raw data as well.

>> >+ {
>> >+ uint16_t tp_num;
>> >+ uint32_t tf_size;
>> >+ unsigned int read_length;
>> >+ unsigned int block;
>> >+
>> >+ /* Read the first six bytes in, which is the tracepoint
>> >+ number and trace frame size. */
>> >+ gotten = target_get_raw_trace_data (buf, offset, 6);
>> >+
>> >+ if (gotten == 0)
>> >+ break;
>> >+ tp_num = (uint16_t)
>> >+ extract_unsigned_integer (&buf[0], 2, byte_order);
>> >+
>> >+ tf_size = (uint32_t)
>> >+ extract_unsigned_integer (&buf[2], 4, byte_order);
>> >+
>> >+ writer->ops->frame_ops->start (writer, tp_num);
>> >+ gotten = 6;
>> >+
>> >+ if (tf_size <= MAX_TRACE_UPLOAD)
>> >+ read_length = tf_size;
>> >+ else
>> >+ {
>> >+ read_length = MAX_TRACE_UPLOAD;
>> >+ gdb_assert (0);
> Leftover? I guess this means you didn't try with a big trace
> frame?
>

I change this part that GDB will skip the frame if it is too big, and
print a warning, like this:

warning: Skip this trace frame because its size (708) is greater than
the size of GDB internal buffer (200)

I set MAX_TRACE_UPLOAD to 200, and run the testsuite. The CTF file is
still correct, but some frames are lost.

>> >+ }
>> >+
>> >+ if (tf_size > 0)
>> >+ {
>> >+ offset += 6;
>> >+ gotten = target_get_raw_trace_data (buf, offset,
>> >+ read_length);
>> >+ gdb_assert (gotten >= read_length);
> An assertion here doesn't seem appropriate, as this is handling input.
> Moreover we have:
>
> /* This is basically a memory transfer, but needs to be its own packet
> because we don't know how the target actually organizes its trace
> memory, plus we want to be able to ask for as much as possible, but
> not be unhappy if we don't get as much as we ask for. */
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> static LONGEST
> remote_get_raw_trace_data (gdb_byte *buf, ULONGEST offset, LONGEST len)
> {
>

The first sentence of header comment is "because we don't know how the
target actually organizes its trace memory", however, we know it here,
because we've got the size of the traceframe. The 'gotten' should be
greater than or equal to the size of the traceframe.

Considering that the remote target may return something wrong, it is too
aggressive to use an assert here. In the new version, I change it to
throwing an error.

>> >+ /* Write to mark the definition part is end. */
> This doesn't parse correctly. Something like:
>
> /* Called to signal the end of the definition part. */
>
> perhaps.
>

Is it incorrect? The basic sentence pattern is "write (an intransitive
verb) to do sth.", and "[that] the definition part is end" is a noun clause.

--
Yao (é½å°§)

[-- Attachment #2: 0001-Refactor-tsave.patch --]
[-- Type: text/x-patch, Size: 24918 bytes --]

gdb:

2013-03-01 Yao Qi <yao@codesourcery.com>

* tracepoint.c (trace_file_writer_xfree): New.
(struct tfile_writer_data): New.
(tfile_can_target_save, tfile_start): New.
(tfile_write_header, tfile_write_regblock_type): New.
(tfile_write_status, tfile_write_uploaded_tsv): New.
(tfile_write_uploaded_tp, tfile_write_definition_end): New.
(tfile_write_raw_data, (tfile_end): New.
(tfile_write_ops): New global variable.
(TRACE_WRITE_R_BLOCK, TRACE_WRITE_M_BLOCK): New macros.
(TRACE_WRITE_V_BLOCK): New macro.
(trace_save): Add extra one parameter WRITER. Make it static.
Use WRITER to writer trace.
(tfile_trace_file_writer_new): New.
(trace_save_command): Caller update.
(trace_save_tfile): Write trace data in TFILE format.
* tracepoint.h (struct trace_frame_write_ops): New.
(struct trace_file_write_ops): New.
(struct trace_file_writer): New.
(trace_save): Remove its declaration.
(trace_save_tfile): Declare it.
* mi/mi-main.c (mi_cmd_trace_save): Call trace_save_tfile
instead of trace_save.
---
gdb/mi/mi-main.c | 2 +-
gdb/tracepoint.c | 559 ++++++++++++++++++++++++++++++++++++++++++------------
gdb/tracepoint.h | 90 +++++++++-
3 files changed, 528 insertions(+), 123 deletions(-)

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 37294e0..206b626 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2495,7 +2495,7 @@ mi_cmd_trace_save (char *command, char **argv, int argc)
filename = argv[0];
}

- trace_save (filename, target_saves);
+ trace_save_tfile (filename, target_saves);
}

void
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 9a80aa3..cc1d742 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2983,90 +2983,323 @@ encode_source_string (int tpnum, ULONGEST addr,
return -1;
}

-extern int trace_regblock_size;
+/* Free trace file writer. */

-/* Save tracepoint data to file named FILENAME. If TARGET_DOES_SAVE is
- non-zero, the save is performed on the target, otherwise GDB obtains all
- trace data and saves it locally. */
+static void
+trace_file_writer_xfree (void *arg)
+{
+ xfree (arg);
+}

-void
-trace_save (const char *filename, int target_does_save)
+/* TFILE trace writer. */
+
+struct tfile_trace_file_writer
{
- struct cleanup *cleanup;
- char *pathname;
- struct trace_status *ts = current_trace_status ();
- int err, status;
+ struct trace_file_writer base;
+
+ /* File pointer to tfile trace file. */
FILE *fp;
- struct uploaded_tp *uploaded_tps = NULL, *utp;
- struct uploaded_tsv *uploaded_tsvs = NULL, *utsv;
- int a;
- char *act;
- LONGEST gotten = 0;
- ULONGEST offset = 0;
-#define MAX_TRACE_UPLOAD 2000
- gdb_byte buf[MAX_TRACE_UPLOAD];
- int written;
+ /* Path name of the tfile trace file. */
+ char *pathname;

- /* If the target is to save the data to a file on its own, then just
- send the command and be done with it. */
- if (target_does_save)
- {
- err = target_save_trace_data (filename);
- if (err < 0)
- error (_("Target failed to save trace data to '%s'."),
- filename);
- return;
- }
+ struct cleanup *cleanup;
+};

- /* Get the trace status first before opening the file, so if the
- target is losing, we can get out without touching files. */
- status = target_get_trace_status (ts);
+/* This is the implementation of trace_file_write_ops method
+ target_save. We just call the generic target
+ target_save_trace_data to do target-side saving. */
+
+static int
+tfile_target_save (struct trace_file_writer *self,
+ const char *filename)
+{
+ int err = target_save_trace_data (filename);
+
+ return (err >= 0);
+}

- pathname = tilde_expand (filename);
- cleanup = make_cleanup (xfree, pathname);
+/* This is the implementation of trace_file_write_ops method
+ start. It creates the trace file FILENAME and registers some
+ cleanups. */

- fp = fopen (pathname, "wb");
- if (!fp)
+static void
+tfile_start (struct trace_file_writer *self, const char *filename)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+
+ writer->pathname = tilde_expand (filename);
+ writer->cleanup = make_cleanup (xfree, writer->pathname);
+ writer->fp = fopen (writer->pathname, "wb");
+ if (writer->fp == NULL)
error (_("Unable to open file '%s' for saving trace data (%s)"),
filename, safe_strerror (errno));
- make_cleanup_fclose (fp);
+ make_cleanup_fclose (writer->fp);
+}
+
+/* This is the implementation of trace_file_write_ops method
+ write_header. Write the TFILE header. */
+
+static void
+tfile_write_header (struct trace_file_writer *self)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+ int written;

/* Write a file header, with a high-bit-set char to indicate a
binary file, plus a hint as what this file is, and a version
number in case of future needs. */
- written = fwrite ("\x7fTRACE0\n", 8, 1, fp);
+ written = fwrite ("\x7fTRACE0\n", 8, 1, writer->fp);
if (written < 1)
- perror_with_name (pathname);
+ perror_with_name (writer->pathname);
+}

- /* Write descriptive info. */
+/* This is the implementation of trace_file_write_ops method
+ write_regblock_type. Write the size of register block. */

- /* Write out the size of a register block. */
- fprintf (fp, "R %x\n", trace_regblock_size);
+static void
+tfile_write_regblock_type (struct trace_file_writer *self, int size)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;

- /* Write out status of the tracing run (aka "tstatus" info). */
- fprintf (fp, "status %c;%s",
+ fprintf (writer->fp, "R %x\n", size);
+}
+
+/* This is the implementation of trace_file_write_ops method
+ write_status. */
+
+static void
+tfile_write_status (struct trace_file_writer *self,
+ struct trace_status *ts)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+
+ fprintf (writer->fp, "status %c;%s",
(ts->running ? '1' : '0'), stop_reason_names[ts->stop_reason]);
if (ts->stop_reason == tracepoint_error)
{
char *buf = (char *) alloca (strlen (ts->stop_desc) * 2 + 1);

bin2hex ((gdb_byte *) ts->stop_desc, buf, 0);
- fprintf (fp, ":%s", buf);
+ fprintf (writer->fp, ":%s", buf);
}
- fprintf (fp, ":%x", ts->stopping_tracepoint);
+ fprintf (writer->fp, ":%x", ts->stopping_tracepoint);
if (ts->traceframe_count >= 0)
- fprintf (fp, ";tframes:%x", ts->traceframe_count);
+ fprintf (writer->fp, ";tframes:%x", ts->traceframe_count);
if (ts->traceframes_created >= 0)
- fprintf (fp, ";tcreated:%x", ts->traceframes_created);
+ fprintf (writer->fp, ";tcreated:%x", ts->traceframes_created);
if (ts->buffer_free >= 0)
- fprintf (fp, ";tfree:%x", ts->buffer_free);
+ fprintf (writer->fp, ";tfree:%x", ts->buffer_free);
if (ts->buffer_size >= 0)
- fprintf (fp, ";tsize:%x", ts->buffer_size);
+ fprintf (writer->fp, ";tsize:%x", ts->buffer_size);
if (ts->disconnected_tracing)
- fprintf (fp, ";disconn:%x", ts->disconnected_tracing);
+ fprintf (writer->fp, ";disconn:%x", ts->disconnected_tracing);
if (ts->circular_buffer)
- fprintf (fp, ";circular:%x", ts->circular_buffer);
- fprintf (fp, "\n");
+ fprintf (writer->fp, ";circular:%x", ts->circular_buffer);
+ fprintf (writer->fp, "\n");
+}
+
+/* This is the implementation of trace_file_write_ops method
+ write_uploaded_tsv. */
+
+static void
+tfile_write_uploaded_tsv (struct trace_file_writer *self,
+ struct uploaded_tsv *utsv)
+{
+ char *buf = "";
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+
+ if (utsv->name)
+ {
+ buf = (char *) xmalloc (strlen (utsv->name) * 2 + 1);
+ bin2hex ((gdb_byte *) (utsv->name), buf, 0);
+ }
+
+ fprintf (writer->fp, "tsv %x:%s:%x:%s\n",
+ utsv->number, phex_nz (utsv->initial_value, 8),
+ utsv->builtin, buf);
+
+ if (utsv->name)
+ xfree (buf);
+}
+
+#define MAX_TRACE_UPLOAD 2000
+
+/* This is the implementation of trace_file_write_ops method
+ write_uploaded_tp. */
+
+static void
+tfile_write_uploaded_tp (struct trace_file_writer *self,
+ struct uploaded_tp *utp)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+ int a;
+ char *act;
+ gdb_byte buf[MAX_TRACE_UPLOAD];
+
+ fprintf (writer->fp, "tp T%x:%s:%c:%x:%x",
+ utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
+ (utp->enabled ? 'E' : 'D'), utp->step, utp->pass);
+ if (utp->type == bp_fast_tracepoint)
+ fprintf (writer->fp, ":F%x", utp->orig_size);
+ if (utp->cond)
+ fprintf (writer->fp,
+ ":X%x,%s", (unsigned int) strlen (utp->cond) / 2,
+ utp->cond);
+ fprintf (writer->fp, "\n");
+ for (a = 0; VEC_iterate (char_ptr, utp->actions, a, act); ++a)
+ fprintf (writer->fp, "tp A%x:%s:%s\n",
+ utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
+ for (a = 0; VEC_iterate (char_ptr, utp->step_actions, a, act); ++a)
+ fprintf (writer->fp, "tp S%x:%s:%s\n",
+ utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
+ if (utp->at_string)
+ {
+ encode_source_string (utp->number, utp->addr,
+ "at", utp->at_string, buf, MAX_TRACE_UPLOAD);
+ fprintf (writer->fp, "tp Z%s\n", buf);
+ }
+ if (utp->cond_string)
+ {
+ encode_source_string (utp->number, utp->addr,
+ "cond", utp->cond_string,
+ buf, MAX_TRACE_UPLOAD);
+ fprintf (writer->fp, "tp Z%s\n", buf);
+ }
+ for (a = 0; VEC_iterate (char_ptr, utp->cmd_strings, a, act); ++a)
+ {
+ encode_source_string (utp->number, utp->addr, "cmd", act,
+ buf, MAX_TRACE_UPLOAD);
+ fprintf (writer->fp, "tp Z%s\n", buf);
+ }
+ fprintf (writer->fp, "tp V%x:%s:%x:%s\n",
+ utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
+ utp->hit_count,
+ phex_nz (utp->traceframe_usage,
+ sizeof (utp->traceframe_usage)));
+}
+
+/* This is the implementation of trace_file_write_ops method
+ write_definition_end. */
+
+static void
+tfile_write_definition_end (struct trace_file_writer *self)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+
+ fprintf (writer->fp, "\n");
+}
+
+/* This is the implementation of trace_file_write_ops method
+ write_raw_data. */
+
+static void
+tfile_write_raw_data (struct trace_file_writer *self, gdb_byte *buf,
+ LONGEST len)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+
+ if (fwrite (buf, len, 1, writer->fp) < 1)
+ perror_with_name (writer->pathname);
+}
+
+/* This is the implementation of trace_file_write_ops method
+ end. */
+
+static void
+tfile_end (struct trace_file_writer *self)
+{
+ struct tfile_trace_file_writer *writer
+ = (struct tfile_trace_file_writer *) self;
+ uint32_t gotten = 0;
+
+ /* Mark the end of trace data. */
+ if (fwrite (&gotten, 4, 1, writer->fp) < 1)
+ perror_with_name (writer->pathname);
+
+ do_cleanups (writer->cleanup);
+}
+
+/* Operations to write trace buffers into TFILE format. */
+
+static struct trace_file_write_ops tfile_write_ops =
+{
+ tfile_target_save,
+ tfile_start,
+ tfile_write_header,
+ tfile_write_regblock_type,
+ tfile_write_status,
+ tfile_write_uploaded_tsv,
+ tfile_write_uploaded_tp,
+ tfile_write_definition_end,
+ tfile_write_raw_data,
+ NULL,
+ tfile_end,
+};
+
+/* Helper macros. */
+
+#define TRACE_WRITE_R_BLOCK(writer, buf, size) \
+ writer->ops->frame_ops->write_r_block ((writer), (buf), (size))
+#define TRACE_WRITE_M_BLOCK(writer, addr, buf, size) \
+ writer->ops->frame_ops->write_m_block ((writer), (addr), (buf), \
+ (size))
+#define TRACE_WRITE_V_BLOCK(writer, num, val) \
+ writer->ops->frame_ops->write_v_block ((writer), (num), (val))
+
+extern int trace_regblock_size;
+
+/* Save tracepoint data to file named FILENAME through WRITER. WRITER
+ determines the trace file format. If TARGET_DOES_SAVE is non-zero,
+ the save is performed on the target, otherwise GDB obtains all trace
+ data and saves it locally. */
+
+static void
+trace_save (const char *filename, struct trace_file_writer *writer,
+ int target_does_save)
+{
+ struct trace_status *ts = current_trace_status ();
+ int status;
+ struct uploaded_tp *uploaded_tps = NULL, *utp;
+ struct uploaded_tsv *uploaded_tsvs = NULL, *utsv;
+
+ ULONGEST offset = 0;
+ gdb_byte buf[MAX_TRACE_UPLOAD];
+ int written;
+ enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
+
+ /* If the target is to save the data to a file on its own, then just
+ send the command and be done with it. */
+ if (target_does_save)
+ {
+ if (!writer->ops->target_save (writer, filename))
+ error (_("Target failed to save trace data to '%s'."),
+ filename);
+ return;
+ }
+
+ /* Get the trace status first before opening the file, so if the
+ target is losing, we can get out without touching files. */
+ status = target_get_trace_status (ts);
+
+ writer->ops->start (writer, filename);
+
+ writer->ops->write_header (writer);
+
+ /* Write descriptive info. */
+
+ /* Write out the size of a register block. */
+ writer->ops->write_regblock_type (writer, trace_regblock_size);
+
+ /* Write out status of the tracing run (aka "tstatus" info). */
+ writer->ops->write_status (writer, ts);

/* Note that we want to upload tracepoints and save those, rather
than simply writing out the local ones, because the user may have
@@ -3081,22 +3314,7 @@ trace_save (const char *filename, int target_does_save)
target_upload_trace_state_variables (&uploaded_tsvs);

for (utsv = uploaded_tsvs; utsv; utsv = utsv->next)
- {
- char *buf = "";
-
- if (utsv->name)
- {
- buf = (char *) xmalloc (strlen (utsv->name) * 2 + 1);
- bin2hex ((gdb_byte *) (utsv->name), buf, 0);
- }
-
- fprintf (fp, "tsv %x:%s:%x:%s\n",
- utsv->number, phex_nz (utsv->initial_value, 8),
- utsv->builtin, buf);
-
- if (utsv->name)
- xfree (buf);
- }
+ writer->ops->write_uploaded_tsv (writer, utsv);

free_uploaded_tsvs (&uploaded_tsvs);

@@ -3106,76 +3324,155 @@ trace_save (const char *filename, int target_does_save)
target_get_tracepoint_status (NULL, utp);

for (utp = uploaded_tps; utp; utp = utp->next)
- {
- fprintf (fp, "tp T%x:%s:%c:%x:%x",
- utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
- (utp->enabled ? 'E' : 'D'), utp->step, utp->pass);
- if (utp->type == bp_fast_tracepoint)
- fprintf (fp, ":F%x", utp->orig_size);
- if (utp->cond)
- fprintf (fp, ":X%x,%s", (unsigned int) strlen (utp->cond) / 2,
- utp->cond);
- fprintf (fp, "\n");
- for (a = 0; VEC_iterate (char_ptr, utp->actions, a, act); ++a)
- fprintf (fp, "tp A%x:%s:%s\n",
- utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
- for (a = 0; VEC_iterate (char_ptr, utp->step_actions, a, act); ++a)
- fprintf (fp, "tp S%x:%s:%s\n",
- utp->number, phex_nz (utp->addr, sizeof (utp->addr)), act);
- if (utp->at_string)
- {
- encode_source_string (utp->number, utp->addr,
- "at", utp->at_string, buf, MAX_TRACE_UPLOAD);
- fprintf (fp, "tp Z%s\n", buf);
- }
- if (utp->cond_string)
- {
- encode_source_string (utp->number, utp->addr,
- "cond", utp->cond_string,
- buf, MAX_TRACE_UPLOAD);
- fprintf (fp, "tp Z%s\n", buf);
- }
- for (a = 0; VEC_iterate (char_ptr, utp->cmd_strings, a, act); ++a)
- {
- encode_source_string (utp->number, utp->addr, "cmd", act,
- buf, MAX_TRACE_UPLOAD);
- fprintf (fp, "tp Z%s\n", buf);
- }
- fprintf (fp, "tp V%x:%s:%x:%s\n",
- utp->number, phex_nz (utp->addr, sizeof (utp->addr)),
- utp->hit_count,
- phex_nz (utp->traceframe_usage,
- sizeof (utp->traceframe_usage)));
- }
+ writer->ops->write_uploaded_tp (writer, utp);

free_uploaded_tps (&uploaded_tps);

/* Mark the end of the definition section. */
- fprintf (fp, "\n");
+ writer->ops->write_definition_end (writer);

/* Get and write the trace data proper. We ask for big blocks, in
the hopes of efficiency, but will take less if the target has
packet size limitations or some such. */
while (1)
{
- gotten = target_get_raw_trace_data (buf, offset, MAX_TRACE_UPLOAD);
+ LONGEST gotten = 0;
+
+ if (writer->ops->write_raw_data != NULL)
+ {
+ gotten = target_get_raw_trace_data (buf, offset,
+ MAX_TRACE_UPLOAD);
+ if (gotten == 0)
+ break;
+ writer->ops->write_raw_data (writer, buf, gotten);
+ }
+ else
+ {
+ uint16_t tp_num;
+ uint32_t tf_size;
+ unsigned int block;
+
+ /* Read the first six bytes in, which is the tracepoint
+ number and trace frame size. */
+ gotten = target_get_raw_trace_data (buf, offset, 6);
+
+ if (gotten == 0)
+ break;
+ tp_num = (uint16_t)
+ extract_unsigned_integer (&buf[0], 2, byte_order);
+
+ tf_size = (uint32_t)
+ extract_unsigned_integer (&buf[2], 4, byte_order);
+
+ /* Skip this trace frame if it is too big. */
+ if (tf_size > MAX_TRACE_UPLOAD)
+ {
+ offset += 6 + tf_size;
+ warning (_("Skip this trace frame because its size (%d)"
+ " is greater than the size of GDB internal"
+ " buffer (%d)"),
+ tf_size, MAX_TRACE_UPLOAD);
+ continue;
+ }
+
+ writer->ops->frame_ops->start (writer, tp_num);
+ gotten = 6;
+
+ if (tf_size > 0)
+ {
+ offset += 6;
+ gotten = target_get_raw_trace_data (buf, offset,
+ tf_size);
+ /* GDB gets to know the size of the trace frame, so GDB
+ should get them. */
+ if (gotten < tf_size)
+ error (_("The target gets incomplete trace data "
+ "(%lld bytes) in one trace frame "
+ "(%d bytes)."), gotten, tf_size);
+
+ gotten = tf_size;
+
+ for (block = 0; block < tf_size; )
+ {
+ gdb_byte block_type = buf[block++];
+
+ switch (block_type)
+ {
+ case 'R':
+ TRACE_WRITE_R_BLOCK (writer, &buf[block],
+ trace_regblock_size);
+ block += trace_regblock_size;
+ break;
+ case 'M':
+ {
+ unsigned short mlen;
+ ULONGEST addr;
+
+ addr = (ULONGEST)
+ extract_unsigned_integer (&buf[block], 8,
+ byte_order);
+ block += 8;
+ mlen = (unsigned short)
+ extract_unsigned_integer (&buf[block], 2,
+ byte_order);
+
+ block += 2;
+ TRACE_WRITE_M_BLOCK (writer, addr,
+ &buf[block], mlen);
+
+ block += mlen;
+ break;
+ }
+ case 'V':
+ {
+ int vnum
+ = (int) extract_signed_integer (&buf[block],
+ 4,
+ byte_order);
+ LONGEST val
+ = extract_signed_integer (&buf[block + 4],
+ 8,
+ byte_order);
+
+ block += (4 + 8);
+ TRACE_WRITE_V_BLOCK (writer, vnum, val);
+ }
+ break;
+ default:
+ error (_("Unknown block type '%c' (0x%x) in"
+ " trace frame"),
+ block_type, block_type);
+ }
+ }
+ }
+
+ writer->ops->frame_ops->end (writer);
+ }
+
if (gotten < 0)
error (_("Failure to get requested trace buffer data"));
/* No more data is forthcoming, we're done. */
if (gotten == 0)
break;
- written = fwrite (buf, gotten, 1, fp);
- if (written < 1)
- perror_with_name (pathname);
offset += gotten;
}

- /* Mark the end of trace data. (We know that gotten is 0 at this point.) */
- written = fwrite (&gotten, 4, 1, fp);
- if (written < 1)
- perror_with_name (pathname);
+ writer->ops->end (writer);
+}
+
+/* Return a trace writer for TFILE format. */
+
+static struct trace_file_writer *
+tfile_trace_file_writer_new (void)
+{
+ struct tfile_trace_file_writer *writer
+ = xmalloc (sizeof (struct tfile_trace_file_writer));
+
+ writer->base.ops = &tfile_write_ops;
+ writer->fp = NULL;
+ writer->pathname = NULL;

- do_cleanups (cleanup);
+ return (struct trace_file_writer *) writer;
}

static void
@@ -3185,6 +3482,7 @@ trace_save_command (char *args, int from_tty)
char **argv;
char *filename = NULL;
struct cleanup *back_to;
+ struct trace_file_writer *writer = NULL;

if (args == NULL)
error_no_arg (_("file in which to save trace data"));
@@ -3205,7 +3503,11 @@ trace_save_command (char *args, int from_tty)
if (!filename)
error_no_arg (_("file in which to save trace data"));

- trace_save (filename, target_does_save);
+ writer = tfile_trace_file_writer_new ();
+
+ make_cleanup (trace_file_writer_xfree, writer);
+
+ trace_save (filename, writer, target_does_save);

if (from_tty)
printf_filtered (_("Trace data saved to file '%s'.\n"), filename);
@@ -3213,6 +3515,21 @@ trace_save_command (char *args, int from_tty)
do_cleanups (back_to);
}

+/* Save the trace data to file FILENAME of tfile format. */
+
+void
+trace_save_tfile (const char *filename, int target_does_save)
+{
+ struct trace_file_writer *writer;
+ struct cleanup *back_to;
+
+ writer = tfile_trace_file_writer_new ();
+ trace_save (filename, writer, target_does_save);
+
+ back_to = make_cleanup (trace_file_writer_xfree, writer);
+ do_cleanups (back_to);
+}
+
/* Tell the target what to do with an ongoing tracing run if GDB
disconnects for some reason. */

diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index b2b9d7c..e48e743 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -205,6 +205,93 @@ struct static_tracepoint_marker
char *extra;
};

+struct trace_file_writer;
+
+/* Operations to write trace frames to a specific trace format. */
+
+struct trace_frame_write_ops
+{
+ /* Write a new trace frame. The tracepoint number of this trace
+ frame is TPNUM. */
+ void (*start) (struct trace_file_writer *self, uint16_t tpnum);
+
+ /* Write an 'R' block. Buffer BUF contains its contents and SIZE is
+ its size. */
+ void (*write_r_block) (struct trace_file_writer *self,
+ gdb_byte *buf, int size);
+
+ /* Write an 'M' block. Buffer BUF contains its contents and LENGTH
+ is its length. ADDR is the start address of collected
+ memory. */
+ void (*write_m_block) (struct trace_file_writer *self,
+ ULONGEST addr, gdb_byte *buf,
+ uint16_t length);
+
+ /* Write a 'V' block. NUM is the trace variable number and VAL is
+ the value of the trace variable. */
+ void (*write_v_block) (struct trace_file_writer *self, int num,
+ LONGEST val);
+
+ /* The end of the trace frame. */
+ void (*end) (struct trace_file_writer *self);
+};
+
+/* Operations to write trace buffers to a specific trace format. */
+
+struct trace_file_write_ops
+{
+ /* Save the data to file or directory NAME of desired format in
+ target side. Return true for success, otherwise return
+ false. */
+ int (*target_save) (struct trace_file_writer *self,
+ const char *name);
+
+ /* Write the trace buffers to file or directory NAME. */
+ void (*start) (struct trace_file_writer *self,
+ const char *name);
+
+ /* Write the trace header. */
+ void (*write_header) (struct trace_file_writer *self);
+
+ /* Write the type of block about registers. SIZE is the size of
+ all registers on the target. */
+ void (*write_regblock_type) (struct trace_file_writer *self,
+ int size);
+
+ /* Write trace status TS. */
+ void (*write_status) (struct trace_file_writer *self,
+ struct trace_status *ts);
+
+ /* Write the uploaded TSV. */
+ void (*write_uploaded_tsv) (struct trace_file_writer *self,
+ struct uploaded_tsv *tsv);
+
+ /* Write the uploaded tracepoint TP. */
+ void (*write_uploaded_tp) (struct trace_file_writer *self,
+ struct uploaded_tp *tp);
+
+ /* Write to mark the definition part is end. */
+ void (*write_definition_end) (struct trace_file_writer *self);
+
+ /* Write the raw data of trace buffer. The content is in BUF and
+ length is LEN. */
+ void (*write_raw_data) (struct trace_file_writer *self,
+ gdb_byte *buf, LONGEST len);
+
+ /* Operations to write trace frames. */
+ struct trace_frame_write_ops *frame_ops;
+
+ /* The end of writing trace buffers. */
+ void (*end) (struct trace_file_writer *self);
+};
+
+/* Trace file writer for a given format. */
+
+struct trace_file_writer
+{
+ struct trace_file_write_ops *ops;
+};
+
extern void parse_static_tracepoint_marker_definition
(char *line, char **pp,
struct static_tracepoint_marker *marker);
@@ -281,7 +368,8 @@ extern void tfind_1 (enum trace_find_type type, int num,
ULONGEST addr1, ULONGEST addr2,
int from_tty);

-extern void trace_save (const char *filename, int target_does_save);
+extern void trace_save_tfile (const char *filename,
+ int target_does_save);

extern struct traceframe_info *parse_traceframe_info (const char *tframe_info);

--
1.7.7.6