From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18435 invoked by alias); 8 Mar 2012 17:07:59 -0000 Received: (qmail 18425 invoked by uid 22791); 8 Mar 2012 17:07:57 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_TD,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; Thu, 08 Mar 2012 17:07:44 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q28H7fPG001850 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 8 Mar 2012 12:07:41 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q28GQJmF028109; Thu, 8 Mar 2012 11:26:20 -0500 Message-ID: <4F58DDAB.7000304@redhat.com> Date: Thu, 08 Mar 2012 17:07:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [patch] Fix PR 13392 : check offset of JMP insn References: <4F561363.4070702@codesourcery.com> <4F56434F.4030107@redhat.com> <4F571B2E.3080707@codesourcery.com> In-Reply-To: <4F571B2E.3080707@codesourcery.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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-03/txt/msg00277.txt.bz2 On 03/07/2012 08:24 AM, Yao Qi wrote: > On 03/07/2012 01:03 AM, Pedro Alves wrote: >> > send a clearer error back to GDB, and we end up with: >> > >> > Target returns error code '.Tracepoint 2 at 0x7ffff67c2579 already exists'. > It is OK for me to let remote target to reports a duplicated tracepoint. I don't know what you mean then. It already does, but with a cryptic E01. > In this version, some changes compared with last one, > > - move `tracepoint already exist' patch out, which can be a separate one, > - remove downloaded tracepoint in target when failed to install, We should also check what happens when we do "enable TRACEPOINT_FOO", and that tracepoints fails to be likewise inserted. Following this direction, we should make sure it stays disabled in the target, but _not_ deleted. > +/* Remove TPOINT from global list. */ > + > +static void > +remove_tracepoint (struct tracepoint *tpoint) > +{ > + struct tracepoint *tp, *tp_prev; > + > + for (tp = tracepoints, tp_prev = NULL; tp && tp != tpoint; > + tp_prev = tp, tp = tp->next) > + ; > + > + if (tp) > + { > + if (tp_prev) > + tp_prev->next = tp->next; > + else > + tracepoints->next = tp->next; This doesn't look correct. If there's no tp_prev, then TP must be TRACEPOINTS, and so you need to do 'tracepoints = tp->next;'. Or just rewrite this as: static int remove_tracepoint (struct tracepoint *todel) { struct tracepoint *tp, **tp_link; tp = tracepoints; tp_link = &tracepoints; while (tp != NULL) { if (tp == todel) { *tp_link = tp->next; xfree (tp); return; } tp_link = &tp->next; tp = *tp_link; } } > /* There may be several tracepoints with the same number (because they > are "locations", in GDB parlance); return the next one after the > given tracepoint, or search from the beginning of the list if the > @@ -2391,6 +2413,8 @@ cmd_qtdp (char *own_buf) > > download_tracepoint (tpoint); > install_tracepoint (tpoint, own_buf); > + if (strncmp (own_buf, "OK", 2) != 0) > + remove_tracepoint (tpoint); own_buf is a \0 terminated string. You can make that a straight strcmp(own_buf, "OK"). (Or change install_tracepoint's return type to int, to signal error.) > + -re "Target returns error code .* too far .*$gdb_prompt $" { > + if [string equal $trace_type "ftrace"] { > + # The target was unable to install the fast tracepoint > + # (e.g., jump pad too far from tracepoint). > + pass "$test (too far)" > + # Skip the rest of the tests. > + return Can you check tab/spaces here, and in the similar places, please? I may have messed that up myself. Otherwise okay. -- Pedro Alves