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

[Doug Evans <dje at google dot com>]

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);
 >  }