Re: [PATCH v3 06/15] Write status to CTF and read.

Doug Evans
2013-03-14T18:06:58Z

Yao Qi writes:
> 2013-03-08 Yao Qi <yao@codesourcery.com>
>
> * ctf.c (CTF_EVENT_ID_STATUS, ctf_save_write_int32): New
> macros.
> (ctf_save_metadata_header): Define new type alias in
> metadata.
> (ctf_write_header): Start a new faked packet for trace status.
> (ctf_write_status): Write trace status to CTF.
> (ctf_write_definition_end): End the faked packet.
> (start_pos): New variable.
> (SET_INT32_FIELD): New macro.
> (ctf_read_status): New.
> (ctf_open): Skip the first faked packet and assert on some
> event types.
> (ctf_trace_find): Set the iterator to the beginning of packet
> including trace frames, instead of the first packet.
> (ctf_get_trace_status): New.
> (init_ctf_ops): Install ctf_get_trace_status to field
> 'to_get_trace_status'.
> @@ -747,15 +798,66 @@ ctf_open_dir (char *dirname)
>
> }
> [...]
> +#define SET_INT32_FIELD(EVENT, SCOPE, VAR, FIELD) \
> + VAR->FIELD = (int) bt_ctf_get_int64 (bt_ctf_get_field (EVENT, \
> + SCOPE, \
> + #FIELD))

Macros like this should be in a do { } while (0).
do { \
...; \
} while (0)

Plus wrap params in parens (except #FIELD of course).

> static void
> ctf_open (char *dirname, int from_tty)
> {
> + struct bt_ctf_event *event;
> + uint32_t event_id;
> + const struct bt_definition *scope;
> +
> target_preopen (from_tty);
> if (!dirname)
> error (_("No CTF directory specified."));
>
> ctf_open_dir (dirname);
>
> + /* Skip the first packet which about the trace status. The first
> + event is "frame". */
> + event = bt_ctf_iter_read_event (ctf_iter);
> + scope = bt_ctf_get_top_level_scope (event, BT_STREAM_EVENT_HEADER);
> + event_id = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
> + gdb_assert (event_id == CTF_EVENT_ID_FRAME);

Question: Will these asserts trigger on bad input data?
[If so, you need to use something else besides gdb_assert.]

> + /* The second event is "status". */
> + gdb_assert (bt_iter_next (bt_ctf_get_iter (ctf_iter)) >= 0);
> + event = bt_ctf_iter_read_event (ctf_iter);
> + scope = bt_ctf_get_top_level_scope (event, BT_STREAM_EVENT_HEADER);
> + event_id = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
> + gdb_assert (event_id == CTF_EVENT_ID_STATUS);
> + ctf_read_status (event, current_trace_status ());
> +
> + /* The third event is "frame". A new packet. */
> + gdb_assert (bt_iter_next (bt_ctf_get_iter (ctf_iter)) >= 0);
> + event = bt_ctf_iter_read_event (ctf_iter);
> + scope = bt_ctf_get_top_level_scope (event, BT_STREAM_EVENT_HEADER);
> + event_id = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
> + gdb_assert (event_id == CTF_EVENT_ID_FRAME);
> +
> + start_pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
> + gdb_assert (start_pos->type == BT_SEEK_RESTORE);
> +
> trace_dirname = xstrdup (dirname);
> push_target (&ctf_ops);
> }