On 03/02/2013 03:22 AM, Tom Tromey wrote: > Yao> + if (fwrite (buf, size, 1, fd) != 1) > Yao> + error (_("Unable to write file for saving trace data (%s)"), > Yao> + safe_strerror (errno)); > > We had this whole thread about using ui-out with patches and everything. > Upthread you said that it was abandoned due to lack of error checking on > write. But I thought that was addressed in the previous thread? > I didn't follow up that discussion, but the the last mail on this topic was sent from Hui and I don't see any discussions from then on. Looks the problem of error checking on write was pointed out but don't have an idea to fix it. > I suppose I don't really care enough to press it. > I don't recall offhand why I thought it was important; maybe some code > duplication, but I don't really see it any more. > OK. > > Yao> + if (!data->tcs.datastream_fd) > > Use "!= NULL". > I think there are some other cases I didn't point out. > > Yao> + /* Tracepoint number. */ > Yao> + ctf_save_write (&data->tcs, (gdb_byte *) &tpnum, 2); > > Does endianness matter? > We set the "byte_order" to the host-endianness in metadata, and write data to CTF datastream file in the host-endianness as well. We don't have to worry about endianness here. > > Yao> add_com ("tsave", class_trace, trace_save_command, _("\ > Yao> Save the trace data to a file.\n\ > Yao> Use the '-r' option to direct the target to save directly to the file,\n\ > Yao> +Use the '-ctf' option to save the data to CTF format,\n\ > Yao> using its own filesystem.")); > > This reads strangely after the change. > Does -ctf direct the target to use its own filesystem? Yes, it is a little bit confusing. > Perhaps rewriting all this into a more "--help" style would be better; > or otherwise at least repeating the final clause. I rewrite it in the new patch. The new patch addresses all the comments. -- Yao (齐尧)