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

["Abid, Hafiz" <Hafiz_Abid at mentor dot com>]

> -----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.

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

>+		  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.