From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13787 invoked by alias); 24 Apr 2012 13:14:14 -0000 Received: (qmail 13715 invoked by uid 22791); 24 Apr 2012 13:14:11 -0000 X-SWARE-Spam-Status: No, hits=-7.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,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; Tue, 24 Apr 2012 13:13:52 +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 q3ODDkfC027642 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 24 Apr 2012 09:13:47 -0400 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 q3ODDiNs023385; Tue, 24 Apr 2012 09:13:45 -0400 Message-ID: <4F96A708.6060804@redhat.com> Date: Tue, 24 Apr 2012 13:18:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 MIME-Version: 1.0 To: Yao Qi CC: Pedro Alves , gdb-patches@sourceware.org, Eli Zaretskii Subject: Re: [PATCH] breakpoint always inserted in record target References: <1334149619-2738-1-git-send-email-yao@codesourcery.com> <4F85C027.3010105@redhat.com> <4F8783C4.60807@codesourcery.com> <4F88482B.4040604@redhat.com> <4F94FB8C.2020906@codesourcery.com> In-Reply-To: <4F94FB8C.2020906@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-04/txt/msg00771.txt.bz2 On 04/23/2012 07:49 AM, Yao Qi wrote:> > +static void > +record_sync_record_breakpoints (struct bp_location *loc, void *data) > +{ > + /* Catchpoints and tracepoints should be skipped. > + TODO: something else should be done for watchpoints, and test cases > + are needed in testsuite/gdb.reverse for tracepoints, catchpoints, and > + watchpoints. */ It's not clear to me that something _is_ indeed needed. If it is, best would be a PR. If not needed, then it's better not to add this comment in the first place, as these tend to remain in the sources forever, misleading. IOW, please drop the comment. > + if (is_tracepoint (loc->owner) || is_catchpoint (loc->owner) > + || is_watchpoint (loc->owner)) > + return; This function should only concert itself about what it knows to handle, instead of skip what it doesn't know about. We don't want to need to add handling for other types here if/when they're added to breakpoint.c in the future. But also, it's really the location's type that matters, not the breakpoint's. target_insert_breakpoint concerns itself with software breakpoints, so this would be better: if (loc->loc_type != bp_loc_software_breakpoint) return; Okay with these changes. > + > + if (loc->inserted) > + { > + struct record_breakpoint *bp = XNEW (struct record_breakpoint); > + > + bp->addr = loc->target_info.placed_address; > + bp->address_space = loc->target_info.placed_address_space; > + > + bp->in_target_beneath = 1; > + > + VEC_safe_push (record_breakpoint_p, record_breakpoints, bp); > + } > +} -- Pedro Alves