From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17827 invoked by alias); 7 Mar 2013 21:29:41 -0000 Received: (qmail 17819 invoked by uid 22791); 7 Mar 2013 21:29:40 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_VF X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 07 Mar 2013 21:29:32 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r27LTVmq009405 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 7 Mar 2013 16:29:31 -0500 Received: from barimba (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r27LTTQZ013523 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Thu, 7 Mar 2013 16:29:30 -0500 From: Tom Tromey To: Yao Qi Cc: Subject: Re: [PATCH 2/5] Save trace into CTF format References: <1361931459-3953-1-git-send-email-yao@codesourcery.com> <1361931459-3953-3-git-send-email-yao@codesourcery.com> <512F38CE.5030802@codesourcery.com> <87621avsam.fsf@fleche.redhat.com> <51332381.6010101@codesourcery.com> Date: Thu, 07 Mar 2013 21:29:00 -0000 In-Reply-To: <51332381.6010101@codesourcery.com> (Yao Qi's message of "Sun, 3 Mar 2013 18:18:41 +0800") Message-ID: <87d2vaew5i.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2.92 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2013-03/txt/msg00315.txt.bz2 >>>>> "Yao" == Yao Qi writes: Yao> +static void Yao> +ctf_save_fwrite (FILE *fd, const gdb_byte *buf, size_t size) Yao> +{ Yao> + if (fwrite (buf, size, 1, fd) != 1) Yao> + error (_("Unable to write file for saving trace data (%s)"), Yao> + safe_strerror (errno)); Yao> +} Why not merge this function with its only caller? Yao> +static void Yao> +ctf_save_fwrite_format (FILE *fd, const char *format, ...) Yao> +{ Yao> + va_list args; Yao> + Yao> + va_start (args, format); Yao> + vfprintf (fd, format, args); This seems like it needs the same error-checking as ctf_save_fwrite; plus maybe (?) the content_size update as well. Yao> +static void Yao> +ctf_write_frame_v_block (struct trace_file_writer *self, Yao> + int num, LONGEST val) Yao> +{ Yao> + struct ctf_trace_file_writer *writer Yao> + = (struct ctf_trace_file_writer *) self; Yao> + uint32_t id = CTF_EVENT_ID_TSV; Yao> + Yao> + /* Event Id. */ Yao> + ctf_save_align_write (&writer->tcs, (gdb_byte *) &id, 4, 4); Yao> + Yao> + /* val. */ Yao> + ctf_save_align_write (&writer->tcs, (gdb_byte *) &val, 8, 8); It seems safer to use a uint64_t intermediary here. Yao> + /* num. */ Yao> + ctf_save_align_write (&writer->tcs, (gdb_byte *) &num, 4, 4); And a uint32_t here. Yao> +/* Save the trace data to dir DIRENAME of ctf format. */ Typo, "DIRNAME". Yao> +void Yao> +trace_save_ctf (const char *dirname, int target_does_save) Yao> +{ Yao> + struct trace_file_writer *writer; Yao> + struct cleanup *back_to; Yao> + Yao> + writer = ctf_trace_file_writer_new (); Yao> + trace_save (dirname, writer, target_does_save); Yao> + back_to = make_cleanup (trace_file_writer_xfree, writer); Yao> + do_cleanups (back_to); I think the make_cleanup and trace_save lines have to be exchanged. Otherwise this doesn't make much sense. Tom