Re: [PATCH 4/6] Add id of TSV into traceframe_info.

[Pedro Alves <palves@redhat.com>]

On 06/13/2013 02:28 AM, Yao Qi wrote:

> diff --git a/gdb/ctf.c b/gdb/ctf.c
> index 278f950..1f6402d 100644
> --- a/gdb/ctf.c
> +++ b/gdb/ctf.c
> @@ -1778,6 +1778,18 @@ ctf_traceframe_info (void)
>  	  def = bt_ctf_get_field (event, scope, "length");
>  	  r->length = (uint16_t) bt_ctf_get_uint64 (def);
>  	}
> +      else if (strcmp (name, "tsv") == 0)
> +	{
> +	  int32_t vnum;
> +	  const struct bt_definition *scope
> +	    = bt_ctf_get_top_level_scope (event,
> +					  BT_EVENT_FIELDS);
> +	  const struct bt_definition *def;
> +
> +	  def = bt_ctf_get_field (event, scope, "num");
> +	  vnum = (int32_t) bt_ctf_get_uint64 (def);
> +	  VEC_safe_push (int, info->tvars, vnum);

The VEC holds "int", so this passing through int32_looks a
little odd.  And why uint64, if the id is always written
as int32 ?

Both are written as int32_t:

...
  /* number */
  ctf_save_write_int32 (&writer->tcs, tsv->number);
...
static void
ctf_write_frame_v_block (struct trace_file_writer *self,
			 int32_t num, uint64_t val)
{
...
  /* num.  */
  ctf_save_align_write (&writer->tcs, (gdb_byte *) &num, 4, 4);
...


BTW, this made me look at the ctf definitions, and I notice that:

  ctf_save_write_metadata (&writer->tcs, "\n");
  ctf_save_write_metadata (&writer->tcs,
			  "event {\n\tname = \"tsv_def\";\n"
			  "\tid = %u;\n\tfields := struct { \n"
			  "\t\tint64_t initial_value;\n"
			  "\t\tint32_t number;\n"
			  "\t\tint32_t builtin;\n"
			  "\t\tchars name;\n"
			  "\t};\n"
			  "};\n", CTF_EVENT_ID_TSV_DEF);

"number" here is int32_t, but here

  ctf_save_write_metadata (&writer->tcs, "\n");
  ctf_save_write_metadata (&writer->tcs,
			   "event {\n\tname = \"tsv\";\n\tid = %u;\n"
			   "\tfields := struct { \n"
			   "\t\tuint64_t val;\n"
			   "\t\tuint32_t num;\n"
			   "\t};\n"
			   "};\n", CTF_EVENT_ID_TSV);


"num" is uint32_t.

Otherwise this is OK.

Thanks,
-- 
Pedro Alves