From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16913 invoked by alias); 15 Jun 2010 19:58:24 -0000 Received: (qmail 16904 invoked by uid 22791); 15 Jun 2010 19:58:23 -0000 X-SWARE-Spam-Status: No, hits=-5.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,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, 15 Jun 2010 19:58:18 +0000 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o5FJwHMo012959 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 15 Jun 2010 15:58:17 -0400 Received: from localhost.localdomain (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o5FJwFUk026992; Tue, 15 Jun 2010 15:58:16 -0400 Message-ID: <4C17DB57.3060107@redhat.com> Date: Tue, 15 Jun 2010 19:58:00 -0000 From: Phil Muldoon User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc12 Lightning/1.0b2pre Thunderbird/3.0.4 MIME-Version: 1.0 To: tromey@redhat.com CC: gdb-patches ml Subject: Re: [python][patch] Inferior and Thread information support. References: <4BFA6E82.3070704@redhat.com> <4C1623C1.6090205@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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-06/txt/msg00352.txt.bz2 On 06/15/2010 07:11 PM, Tom Tromey wrote: >>>>>> "Phil" == Phil Muldoon writes: > > Phil> I've no complaint to using obstacks. This function basically > Phil> wraps/tidies the existing code that was just coded directly in a loop > Phil> in parse_find_args. That code just realloc'd by a factor of two whenever > Phil> the buffer was too small. This code is exactly the same, except it > Phil> has been squirrelled away in a function. So we are not introducing or > Phil> adding any more growable types in this patch, just moving the code > Phil> bits that already existed into function. I'm not adverse to changing > Phil> that code to use obstacks, that being said! > > Just for the record -- ordinarily I try not to request cleanups to > existing code as part of a new patch. It is nice to get cleanups, and > if you want to do them (or if there is a reason for them beyond mere > tidiness) then that is great. But feel free to push back if I've > erroneously reviewed the context and not the patch. We talked a little about this on irc. I think your original instincts were totally right. I had second thoughts about it. We're going through this bit of code, so lets just fix it -- and I'm happy to do it. I did a basic conversion to obstacks today; I'll refine it for the patch later. I don't see it being a huge issue. > > Phil> + /* Find inferior_object for the given PID. */ > Phil> + for (inf_entry = &gdbpy_inferior_list; *inf_entry != NULL; > Phil> + inf_entry = &(*inf_entry)->next) > Phil> + if ((*inf_entry)->inf_obj->inferior->pid == inf->pid) > Phil> + break; >> > Tom> It seems strange to compare the pid fields when we could just compare > Tom> the inferior objects themselves. > > Phil> Do you mean using the Python object's cmp inbuilt method here? > > No, I'm just curious why that can't be more simply written: > > if ((*inf_entry)->inf_obj == inf) When I ported this code from Archer I thought the (t)pid equality was fine. But as in any porting effort you take the sum of several authors and decide to rewrite/port what you have. I thought that this equality was ok. OTOH I don't see why either (on your original question). I'll investigate further and report back. Cheers, Phil