From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22744 invoked by alias); 30 Sep 2013 11:50:34 -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 22731 invoked by uid 89); 30 Sep 2013 11:50:33 -0000 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 30 Sep 2013 11:50:33 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,RDNS_NONE,SPF_HELO_FAIL autolearn=no version=3.3.2 X-HELO: relay1.mentorg.com Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1VQbzf-000520-Jj from Luis_Gustavo@mentor.com ; Mon, 30 Sep 2013 04:50:27 -0700 Received: from NA1-MAIL.mgc.mentorg.com ([147.34.98.181]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Mon, 30 Sep 2013 04:50:27 -0700 Received: from [172.30.6.33] ([172.30.6.33]) by NA1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 30 Sep 2013 04:50:27 -0700 Message-ID: <5249657B.8030505@codesourcery.com> Date: Mon, 30 Sep 2013 11:50:00 -0000 From: Luis Machado Reply-To: lgustavo@codesourcery.com User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: Pedro Alves CC: Joel Brobecker , "'gdb-patches@sourceware.org'" Subject: Re: [PATCH, cleanup] Standardize access to ptid References: <523B7A79.1060901@codesourcery.com> <20130926125258.GA3198@adacore.com> <52442F4E.8000808@codesourcery.com> <52444E6A.2060104@redhat.com> <5248607C.6030909@codesourcery.com> <52487534.3000609@redhat.com> In-Reply-To: <52487534.3000609@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2013-09/txt/msg01007.txt.bz2 On 29-09-2013 15:45, Pedro Alves wrote: > On 09/29/2013 06:16 PM, Luis Machado wrote: > >>>> @@ -33,6 +33,10 @@ >>>> ptid_get_lwp - Fetch the lwp component of a ptid. >>>> ptid_get_tid - Fetch the tid component of a ptid. >>>> ptid_equal - Test to see if two ptids are equal. >>>> + ptid_is_pid - Test if a ptid's pid component is non-zero. >>> >>> No, that's not right: >>> >>> /* Returns true if PTID represents a process. */ >>> >>> int >>> ptid_is_pid (ptid_t ptid) >>> { >>> if (ptid_equal (minus_one_ptid, ptid)) >>> return 0; >>> if (ptid_equal (null_ptid, ptid)) >>> return 0; >>> >>> return (ptid_get_lwp (ptid) == 0 && ptid_get_tid (ptid) == 0); >>> } >>> >>> So this only returns true iff the ptid looks like (pid,0,0). >>> (ptid_is_pid on (pid,lwp,0) returns false, for example.) >>> This is considered a ptid that identifies the whole PID process (the >>> whole thread group in Linux speak). Both the core and the targets >>> use and agree on this. >> >> I've changed the description to the following: >> >> "Test if a ptid looks like (pid, 0, 0)." >> >> Seems to clearly state what is being checked. > > Sounds fine, thanks. > >>> ... > >> All of this makes sense to me, but perhaps we should introduce such a >> change later on? After the cleanup possibly, since this will require >> changes in places of the code that deal with various subsystems of GDB. > > Yes, I was just doing a brain dump. I'm not suggesting to actually > do it now. And certainly not ever as part of this patch. > >>> With that in mind, I think I'd prefer renaming these >>> new "is" functions as: >>> >>> ptid_is_lwp -> ptid_lwp_p >>> ptid_is_tid -> ptid_tid_p >>> >>> (or at least ptid_has_lwp, though the _p variant has >>> precedent in the frame stuff, and it feels to me that frame_ids >>> and ptids are at about the same conceptual level.) >> >> I'm happy with ptid_lwp_p and ptid_tid_p. > > Thanks. > >>> And I'm also don't really like the "ptid_is_invalid" function that much. >>> minus_one_ptid or null_ptid aren't really always invalid. They have >>> special meanings as either invalid, terminator, or as wildcard depending >>> on context. See e.g, how frame_id_p returns true to wildcard frame ids, >>> and the special outer_frame_id (although that one should die. >>> But with the above suggestion, I don't think the function >>> would end up with any use left, so it could just be dropped. But I suppose >>> I'll just get used to it if it stays. ;-) But, if it stays, please, >>> please, invert its logic, getting rid of the double >>> negative ("if (!ptid_is_invalid ()"). >> >> I thought about ptid_special_p or ptid_is_special to check for both > > Yeah, good one. I would have liked that naming more. > >> null_ptid and minus_one_ptid, but this check would only be used (for now >> at least) in the ptid.c file. Maybe not worth the effort, so i left it out. > > Yeah. > > I only skimmed most of the new patch, focused mostly on ptid.c/ptid.h, and > on the places is_lwp / is_thread used to be used, and, I'm fine with this > version. Thanks a lot! > Checked in. Thanks! Luis