From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10061 invoked by alias); 13 Dec 2010 02:47:25 -0000 Received: (qmail 10053 invoked by uid 22791); 13 Dec 2010 02:47:24 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 13 Dec 2010 02:47:20 +0000 Received: (qmail 26280 invoked from network); 13 Dec 2010 02:47:17 -0000 Received: from unknown (HELO ?192.168.0.102?) (yao@127.0.0.2) by mail.codesourcery.com with ESMTPA; 13 Dec 2010 02:47:17 -0000 Message-ID: <4D05892D.8030800@codesourcery.com> Date: Mon, 13 Dec 2010 02:47:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.13) Gecko/20101208 Thunderbird/3.1.7 MIME-Version: 1.0 To: Jan Kratochvil CC: gdb-patches@sourceware.org Subject: Re: [patch 3/4] hw watchpoints made multi-inferior References: <20101206111443.GE27176@host0.dyn.jankratochvil.net> <4CFDBFC3.5000803@codesourcery.com> <20101211051554.GC10298@host0.dyn.jankratochvil.net> In-Reply-To: <20101211051554.GC10298@host0.dyn.jankratochvil.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 2010-12/txt/msg00169.txt.bz2 On 12/11/2010 01:15 PM, Jan Kratochvil wrote: > >>> * linux-nat.c (linux_nat_iterate_watchpoint_lwps): Fix >>> iterate_over_lwps FILTER. >> >> What is FILTER? > > `filter' is name of the first parameter of iterate_over_lwps. > >> Why capitalized? > > I follow: > > http://sourceware.org/ml/gdb-patches/2008-09/msg00374.html > On Wed, 17 Sep 2008 20:03:37 +0200, Daniel Jacobowitz wrote: > # You capitalize FOO when you mean "the value of a variable named foo", but the > # name of the variable is still "foo". > > But I haven't found such rule in GNU Coding Standards (GCS) now. But some > keywords are capitalized in GCS. > IMO, this rule is for comments of function header, and I don't see it in ChangeLog entries before. Thanks for this explanation. It looks OK to me. >>> + if (old_loc->pspace != current_program_space) >>> + continue; >> >> code indent problem? Put extra three spaces in front of "continue". > > There is before `continue;'. The tab should be there for any 8 spaces > according to GNU indent options present in GCS. > > It just gets displayed wrong in the patch file due to the first column from > diff. I do not know an easy solution for it. Rather well formatted code > using >=4 blocks indentation looks wrong to me (as it may not be using tabs). > OK, no problem. > >>> +static void >>> +i386_inferior_data_cleanup (struct inferior *inf, void *arg) >>> +{ >>> + struct i386_inferior_data *inf_data = arg; >>> + >>> + xfree (inf_data); >>> +} >> >> Can't we 'xfree (arg);' directly? > > This was discussed here before at least in: > http://sourceware.org/ml/gdb-patches/2010-05/msg00162.html > > The problem is primarily GDB still has not switched to C++ with templates > where ARG could have the right type instead of `void *'. > > With the `void *' cast I find it both more readable to see what the parameter > in fact is. And I also find it safer is someone needs to access some members > of `struct i386_inferior_data' in the future. Having to re-cast `void *' to > the right type during such code changes can be IMO more fragile than to just > write it down once when one implements both the caller and the callback. > > And it is just the simpler stupid writing of code when one does not have to > think about it much and it just works. > > Unless decided otherwise so far I consider this style is agreed upon for GDB. Oh, yes. I didn't know it before. Thanks for your explanation. >>> +if ![runto_main] { >> >> Please add `perror "Couldn't run ${testfile}"' >> >>> + return >>> +} > > In any case runto_main returns 0 some kind of error message has been already > printed. Or do I miss some case? > No. Let's keep it as it is now. I have no other comments to your new patch. -- Yao (齐尧)