From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10170 invoked by alias); 17 Dec 2012 20:43:26 -0000 Received: (qmail 10161 invoked by uid 22791); 17 Dec 2012 20:43:23 -0000 X-SWARE-Spam-Status: No, hits=-6.2 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_CP,T_RP_MATCHES_RCVD 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; Mon, 17 Dec 2012 20:43:14 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBHKhCFT000972 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 17 Dec 2012 15:43:12 -0500 Received: from host2.jankratochvil.net (ovpn-116-39.ams2.redhat.com [10.36.116.39]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id qBHK4ulg008543 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Mon, 17 Dec 2012 15:04:59 -0500 Date: Mon, 17 Dec 2012 20:43:00 -0000 From: Jan Kratochvil To: markus.t.metzger@intel.com Cc: palves@redhat.com, tromey@redhat.com, kettenis@gnu.org, gdb-patches@sourceware.org, markus.t.metzger@gmail.com Subject: Re: [patch v6 08/12] gdbserver, btrace: add generic btrace support Message-ID: <20121217200456.GH14232@host2.jankratochvil.net> References: <1355760101-26237-1-git-send-email-markus.t.metzger@intel.com> <1355760101-26237-9-git-send-email-markus.t.metzger@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1355760101-26237-9-git-send-email-markus.t.metzger@intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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: 2012-12/txt/msg00601.txt.bz2 On Mon, 17 Dec 2012 17:01:37 +0100, markus.t.metzger@intel.com wrote: [...] > +/* Handle the "Qbtrace" packet. */ > + > +static int > +handle_btrace_general_set (char *own_buf) > +{ > + struct thread_info *thread; > + const char *err; > + char *op; > + > + if (strncmp ("Qbtrace:", own_buf, strlen ("Qbtrace:")) != 0) > + return 0; > + > + op = own_buf + strlen ("Qbtrace:"); > + > + if (!target_supports_btrace ()) > + { > + strcpy (own_buf, "E.Target does not support branch tracing."); > + return 1; > + } > + > + if (ptid_equal (general_thread, null_ptid) > + || ptid_equal (general_thread, minus_one_ptid)) > + { > + strcpy (own_buf, "E.Must select a single thread."); > + return -1; > + } > + > + thread = find_thread_ptid (general_thread); > + if (thread == NULL) > + { > + strcpy (own_buf, "E.No such thread."); > + return -1; > + } > + > + err = NULL; > + > + if (strncmp ("on", op, strlen ("on")) == 0) Here > + err = handle_btrace_enable (thread); > + else if (strncmp ("off", op, strlen ("off")) == 0) and here should be just strcmp, otherwise it falsely matches for example future keyword "official" or whatever. > + err = handle_btrace_disable (thread); > + else > + err = "E.Bad Qbtrace operation. Use on or off."; > + > + if (err != 0) > + strcpy (own_buf, err); > + else > + write_ok (own_buf); > + > + return 1; > +} > + [...] > +/* Handle the "qbtrace" packet. */ > + > +static int > +handle_btrace_query (char *own_buf) > +{ > + if (strncmp ("qbtrace:", own_buf, strlen ("qbtrace:")) == 0) > + { > + char *ptid_str = own_buf + strlen ("qbtrace:"); > + ptid_t ptid = read_ptid (ptid_str, NULL); > + struct thread_info *thread = find_thread_ptid (ptid); > + > + if (thread == NULL) > + strcpy (own_buf, "E.No such thread."); Incorrect indentation. > + else if (thread->btrace == NULL) > + strcpy (own_buf, "E.Btrace not enabled."); Incorrect indentation. > + else > + strcpy (own_buf, > + (target_btrace_has_changed (thread->btrace) ? "yes" : "no")); Use tabs, not spaces. Also below and everywhere in this patch (or patchset?) [...] > --- a/gdb/gdbserver/target.h > +++ b/gdb/gdbserver/target.h [...] > +#define target_supports_btrace() \ > + (the_target->supports_btrace ? (*the_target->supports_btrace) () : 0) > + > +#define target_enable_btrace(ptid) \ Use tabs for the \ backslash indentation. > + (*the_target->enable_btrace) (ptid) > + > +#define target_disable_btrace(tinfo) \ > + (*the_target->disable_btrace) (tinfo) > + > +#define target_btrace_has_changed(tinfo) \ > + (*the_target->btrace_has_changed) (tinfo) > + > +#define target_read_btrace(tinfo, buffer) \ > + (*the_target->read_btrace) (tinfo, buffer) > + > /* Start non-stop mode, returns 0 on success, -1 on failure. */ > > int start_non_stop (int nonstop); > -- > 1.7.6.5 Thanks, Jan