From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23729 invoked by alias); 25 Mar 2013 18:10:07 -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 23628 invoked by uid 89); 25 Mar 2013 18:09:59 -0000 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Mon, 25 Mar 2013 18:09:59 +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 r2PI9um8030001 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 25 Mar 2013 14:09:56 -0400 Received: from valrhona.uglyboxes.com (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 r2PI9sCO016349 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Mon, 25 Mar 2013 14:09:56 -0400 Message-ID: <515092F2.2000307@redhat.com> Date: Mon, 25 Mar 2013 19:58:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120605 Thunderbird/13.0 MIME-Version: 1.0 To: Hui Zhu CC: Tom Tromey , Hui Zhu , gdb-patches ml Subject: Re: [PATCH] Fix create pending breakpoint handle extra_string issue if not parse_condition_and_thread References: <514E8D6C.2010606@mentor.com> <514EEB43.6040101@redhat.com> <87620ftn9f.fsf@fleche.redhat.com> In-Reply-To: <87620ftn9f.fsf@fleche.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2013-03/txt/msg00948.txt.bz2 On 03/25/2013 10:14 AM, Tom Tromey wrote: >>>>>> "Hui" == Hui Zhu writes: > > Hui> I am sorry that what you care about is the issue that affect the mi. > Hui> But my patch is for the issue inside the function create_breakpoint. Actually I /was/ talking about create_breakpoint. As you stated, the only way to demonstrate the problem is via MI, so that's what I used to demonstrate how I think the situation should be handled. Here's a patch which does exactly what I consider the "right" way to react to having both cond_string and a condition inside arg: Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.747 diff -u -p -r1.747 breakpoint.c --- breakpoint.c 20 Mar 2013 22:17:18 -0000 1.747 +++ breakpoint.c 25 Mar 2013 17:59:36 -0000 @@ -9659,6 +9659,11 @@ create_breakpoint (struct gdbarch *gdbar extra_string = xstrdup (extra_string); make_cleanup (xfree, extra_string); } + else if (*arg != '\000') + { + extra_string = xstrdup (arg); + make_cleanup (xfree, extra_string); + } } ops->create_breakpoints_sal (gdbarch, &canonical, lsal, > In this case, it seems to me that the API must be a bad one. Yes, that API extension was a horribly implemented (quick and dirty), but create_breakpoint is a bit of a mess, since it not only has to deal with setting breakpoints (of various varieties), it also has to deal with parsing user input. I'm not a fan of this (too common) paradigm. > Can't we just tell callers, "don't do that"? > To me it seems like a pathological case. We can certainly enforce this, as my patchlet above demonstrates: -break-insert -c "argc > 1" "main if argc > 2" ^error,msg="Garbage 'if argc > 2' at end of command" Keith