Re: [PATCH] Add CTF support to GDB [1/4] Add "-ctf" to tsave command

[Hui Zhu <teawater@gmail.com>]

On Wed, Nov 21, 2012 at 2:47 PM, Abid, Hafiz <Hafiz_Abid@mentor.com> wrote:
>
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] On Behalf Of Zhu, Hui
>> Sent: Wednesday, November 21, 2012 1:45 AM
>> To: gdb-patches@sourceware.org
>> Subject: [PATCH] Add CTF support to GDB [1/4] Add "-ctf" to tsave
>> command
>>
>> This patch is for the CTF write.
>> It add "-ctf" to tsave command.  With this option, tsave can save
>> current trace frame to CTF file format.
>>
>> Thanks,
>> Hui
>>
>> 2012-11-20  Hui Zhu  <hui_zhu@mentor.com>
>>
>>       * Makefile.in (REMOTE_OBS): Add ctf.o.
>>       (SFILES): Add ctf.c.
>>       (HFILES_NO_SRCDIR): Add ctf.h.
>>       * ctf.c, ctf.h: New files.
> Some more comments in these file will help understand the code better. Most of the function don't have any comments in the start so it is difficult to follow.
>
>>+      case TYPE_CODE_INT:
>>+      /* XXX: didn't check !TYPE_NOSIGN (t->type) because char is
>>+         signed in C.  */
> This statement is not completely true. I think it is left to implementation to decide the signed-ness of char.

I think type struct already include all the info of this type.   I
think it because this comment is not clear.  I will make it clear in
the new patch.

>
>>+      /* XXX: no sure about variable_length
>>+       and need set is_variable_length if need.  */
> This comment is confusing.

I will update it.

Thanks,
Hui

>
>>+                warning (_("\
>>+Traceframe %d is dropped because try to get the value of \"%s\" got error: %s"),
>
>>+      warning (_("\
>>+Not save \"%s\" of tracepoint %d to ctf file because get its value fail: %s"),
> Many error and warning messages need some improvement.
>
>>       * mi/mi-main.c (ctf.h): New include.
>>       (mi_cmd_trace_save): Add "-ctf".
>>       * tracepoint.c (ctf.h): New include.
>>       (collect_pseudocommand): Remove static.
>>       (trace_save_command): Add "-ctf".
>>       (_initialize_tracepoint): Ditto.
>>       * tracepoint.h (collect_pseudocommand): Add extern.