From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15192 invoked by alias); 9 Apr 2013 15:53:47 -0000 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 Received: (qmail 15183 invoked by uid 89); 9 Apr 2013 15:53:46 -0000 X-Spam-SWARE-Status: No, score=-6.8 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD autolearn=ham version=3.3.1 Received: from mail-oa0-f74.google.com (HELO mail-oa0-f74.google.com) (209.85.219.74) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 09 Apr 2013 15:53:45 +0000 Received: by mail-oa0-f74.google.com with SMTP id k14so1786993oag.3 for ; Tue, 09 Apr 2013 08:53:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:mime-version:content-type:content-transfer-encoding :message-id:date:to:cc:subject:in-reply-to:references:x-mailer :x-gm-message-state; bh=XN5vC5ajTM+mc/AJ5BvdSRemAkTk3AWDcjbOwNeEINI=; b=fEA+lIDFt/32b+XV7f8rzzj5cLETYyKHlbXA9IgnnDTs6q2xICWsb/2/8Zx3wPs5KD 3JtadF6DKos+FhOyDFCDFLx2xTqKWhAlgGsnYUv8CJSv8gpyII47N0WXmhLzHbK64BNy W1a8WAzbLlrRwQPewtI0tnMXQqkkpGgT+tY0sPrG/N44Nd1k0x/LM340WpvJ2YJJQEC8 l4uloHQfnBKIsP4iNxxdky+pqVkEiGpu2ha1IleYn3o5XU5qpIS4reKxw1wJdZU2gD3r L9fcKE+/DwXx5KD2y3wicemE8USkJ6RUim/6OVWzIYE52qRHjzKf7StkQhnB23080TSI q+4w== X-Received: by 10.42.47.140 with SMTP id o12mr19772220icf.32.1365522824228; Tue, 09 Apr 2013 08:53:44 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id x4si2167114igl.2.2013.04.09.08.53.44 (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Tue, 09 Apr 2013 08:53:44 -0700 (PDT) Received: from ruffy2.mtv.corp.google.com (ruffy2.mtv.corp.google.com [172.17.128.107]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 16B3931C2D7; Tue, 9 Apr 2013 08:53:42 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=unknown Content-Transfer-Encoding: 7bit Message-ID: <20836.14726.472470.645674@ruffy2.mtv.corp.google.com> Date: Tue, 09 Apr 2013 18:41:00 -0000 To: Yao Qi Cc: Tom Tromey , gdb-patches Subject: Re: [PATCH v3 03/15] Read CTF by the ctf target In-Reply-To: <5163D6FE.1090106@codesourcery.com> References: <1362800844-27940-1-git-send-email-yao@codesourcery.com> <1362800844-27940-4-git-send-email-yao@codesourcery.com> <871ubjawq8.fsf@fleche.redhat.com> <20802.2886.914505.51784@ruffy2.mtv.corp.google.com> <51501E65.1020801@codesourcery.com> <20816.28886.574691.604457@ruffy2.mtv.corp.google.com> <51514D75.3070402@codesourcery.com> <20821.49407.164383.945535@ruffy2.mtv.corp.google.com> <51626EAC.3090208@codesourcery.com> <20834.63301.678122.134757@ruffy2.mtv.corp.google.com> <5163D6FE.1090106@codesourcery.com> X-Gm-Message-State: ALoCoQncjGHxOhiQDGA8+7TkgNhwCnnBEUJmNjUZnKbcO9qEALIplqY0B+ss8JNACklwac0+BS5CvdoYmpeTglivMPeU0tyIxoD2Lpv3wr8c0ngYL8tjTI2Z3iThnvcmH3zlnF8Mfi6AhXAC0UCJE4irs9HevyfMzsZP46kywbE0Hz0HzJMNa9dxhVByc59voh2ZBm8W4kvpaUaXbZVNYUUrXk4ba+tbiw== X-SW-Source: 2013-04/txt/msg00240.txt.bz2 Yao Qi writes: > On 04/09/2013 12:58 AM, Doug Evans wrote: > >> + if (ret < 0) > > > + { > > > + ctf_destroy (); > > > + error (_("Unable to use libbabeltrace open \"%s\""), dirname); > > > > The wording of this error message (here and below) is awkward. > > I don't know libbabeltrace enough to know if this is accurate, but > > it reads better. > > > > error (_("Unable to use libbabeltrace on \"%s\""), dirname); > > > > or > > > > error (_("Unable to use libbabeltrace on directory \"%s\""), dirname); > > I changed the message to > > error (_("Unable to use libbabeltrace on directory \"%s\""), > dirname); > > > > > Also, IWBN to add a more specific reason for the failure, > > but I'm not sure how easy that would be. > > Is there a bt_foo function akin to strerror? > > > > Unfortunately, there isn't such function. > > > > > + > > > + if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0) > > > + break; > > > + } > > > + > > > + /* Restore the position. */ > > > + bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos); > > > + > > > > Remove blank line. > > > > > +} > > Removed. > > > > > + > > > + scope = bt_ctf_get_top_level_scope (event, > > > + BT_EVENT_FIELDS); > > > + > > > + def = bt_ctf_get_field (event, scope, "address"); > > > + maddr = bt_ctf_get_uint64 (def); > > > + def = bt_ctf_get_field (event, scope, "length"); > > > + mlen = (uint16_t) bt_ctf_get_uint64 (def); > > > > I don't know libbabeltrace, I'm assuming length can be at most 16 bits. > > > > Right. "length" is defined as uint16_t in metadata, on the other hand, > "length" is got from the remote target, which is 16-bit as well. > > > > + > > > +/* This is the implementation of target_ops method to_traceframe_info. > > > + Iterate the events whose name is "memory", in current > > > + frame, extract memory range information, and return them in > > > + traceframe_info. */ > > > + > > > +static struct traceframe_info * > > > +ctf_traceframe_info (void) > > > +{ > > > + struct traceframe_info *info = XCNEW (struct traceframe_info); > > > + const char *name; > > > + struct bt_iter_pos *pos; > > > + > > > + gdb_assert (ctf_iter != NULL); > > > + /* Save the current position. */ > > > + pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter)); > > > + gdb_assert (pos->type == BT_SEEK_RESTORE); > > > + > > > + do > > > + { > > > + struct bt_ctf_event *event > > > + = bt_ctf_iter_read_event (ctf_iter); > > > + > > > + name = bt_ctf_event_name (event); > > > + > > > + if (name == NULL || strcmp (name, "register") == 0 > > > + || strcmp (name, "frame") == 0) > > > + ; > > > + else if (strcmp (name, "memory") == 0) > > > + { > > > + const struct bt_definition *scope > > > + = bt_ctf_get_top_level_scope (event, > > > + BT_EVENT_FIELDS); > > > + const struct bt_definition *def; > > > + struct mem_range *r; > > > + > > > + r = VEC_safe_push (mem_range_s, info->memory, NULL); > > > + def = bt_ctf_get_field (event, scope, "address"); > > > + r->start = bt_ctf_get_uint64 (def); > > > + > > > + def = bt_ctf_get_field (event, scope, "length"); > > > + r->length = (uint16_t) bt_ctf_get_uint64 (def); > > > + } > > > + else > > > + warning (_("Unhandled trace block type (%s) " > > > + "while building trace frame info."), > > > + name); > > > > These days we prefer multi-line single statements for if/else/etc. > > to be wrapped in braces. i.e., > > > > { > > warning (_("Unhandled trace block type (%s) " > > "while building trace frame info."), > > name); > > } > > > > Fixed. I knew that single statement with comments should be warped by > braces, like: > > if (foo) > /* Comments. */ > bar (); > > after reading GDB internals, I realize that "warning message in > multiple lines" falls into this category as well. You are right. > > > > + > > > +/* module initialization */ > > > > blank line > > > > > +void > > > +_initialize_ctf (void) > > Fixed. > > -- > Yao ( ) > > gdb: > > 2013-04-09 Hui Zhu > Yao Qi > > * configure.ac: Check libbabeltrace is installed. > * config.in: Regenerate. > * configure: Regenerate. > * Makefile.in (LIBBABELTRACE): New. > (CLIBS): Add LIBBABELTRACE. > * ctf.c (ctx, ctf_iter, trace_dirname): New. > (ctf_destroy, ctf_open_dir, ctf_open): New. > (ctf_close, ctf_files_info): New. > (ctf_fetch_registers, ctf_xfer_partial): New. > (ctf_get_trace_state_variable_value): New. > (ctf_get_tpnum_from_frame_event): New. > (ctf_get_traceframe_address): New. > (ctf_trace_find, ctf_has_stack): New. > (ctf_has_registers, ctf_traceframe_info, init_ctf_ops): New. > (_initialize_ctf): New. > * tracepoint.c (get_tracepoint_number): New > (struct traceframe_info, trace_regblock_size): Move it to ... > * tracepoint.h: ... here. > (get_tracepoint_number): Declare it. Thanks. The patch is ok.