From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32207 invoked by alias); 19 Sep 2013 12:34:11 -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 32196 invoked by uid 89); 19 Sep 2013 12:34:11 -0000 Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 19 Sep 2013 12:34:11 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id AB3E4116639; Thu, 19 Sep 2013 08:34:23 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id udL3FfvnOBqo; Thu, 19 Sep 2013 08:34:23 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 814F01165FF; Thu, 19 Sep 2013 08:34:23 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id E64D4E030E; Thu, 19 Sep 2013 05:34:07 -0700 (PDT) Date: Thu, 19 Sep 2013 12:34:00 -0000 From: Joel Brobecker To: Luis Machado Cc: "'gdb-patches@sourceware.org'" Subject: Re: [PATCH] Move code to common/ptid.h Message-ID: <20130919123407.GQ3132@adacore.com> References: <523AEB33.9080100@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <523AEB33.9080100@codesourcery.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2013-09/txt/msg00672.txt.bz2 > 2013-09-19 Luis Machado > > * common/ptid.h (GET_LWP, GET_PID): Moved from > linux-nat.h. > (is_lwp, BUILD_LWP): Likewise. > * linux-nat.h (GET_LWP, GET_PID): Moved to > common/ptid.h. > (is_lwp, BUILD_LWP): Likewise. No real objection from me. But I am wondering about the usefulness of these macros, now that they are straight mapping to functions of the same name. Some thoughts for the long term below. Do not feel like you should take them on yourself. Just sharing them, and seeing how people feel about that. I may take that on myself. If there is a general agreement, though, then perhaps the patch you are suggestion is a step in the wrong direction... In the past, before we had ptid_t, I have always found the (target- specific?) effect of these macros to be a little obscure, and I am still suffering from those effects. Getting over it is not the biggest challenge I have faced in my life :-), but if the macros are not really necessary, how about slowly transitioning them out in favor of using the functions directly, at least for GET_LWP and GET_PID. I get "is_lwp", and we could either keep that as a macro, or define a function. For BUILD_LWP, I personally don't see an advantage to having this macro or function, but perhaps others might prefer having the hint that we're building an LWP directly in the macro/function name. I would define a function, though, Eg ptid_build_lwp (pid, lwp). Cheers, Luis. > > gdb/common/ptid.h | 6 ++++++ > gdb/linuxt.h | 5 ----- > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/gdb/common/ptid.h b/gdb/common/ptid.h > index fefe8b6..ceafd02 100644 > --- a/gdb/common/ptid.h > +++ b/gdb/common/ptid.h > @@ -20,6 +20,12 @@ > #ifndef PTID_H > #define PTID_H > > +/* Helper macros. */ > +#define GET_LWP(ptid) ptid_get_lwp (ptid) > +#define GET_PID(ptid) ptid_get_pid (ptid) > +#define is_lwp(ptid) (GET_LWP (ptid) != 0) > +#define BUILD_LWP(lwp, pid) ptid_build (pid, lwp, 0) > + > /* The ptid struct is a collection of the various "ids" necessary > for identifying the inferior. This consists of the process id > (pid), thread id (tid), and other fields necessary for uniquely > diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h > index 044f646..0fc68ef 100644 > --- a/gdb/linux-nat.h > +++ b/gdb/linux-nat.h > @@ -110,11 +110,6 @@ extern struct lwp_info *lwp_list; > (LP) != NULL; \ > (LP) = (LP)->next) > > -#define GET_LWP(ptid) ptid_get_lwp (ptid) > -#define GET_PID(ptid) ptid_get_pid (ptid) > -#define is_lwp(ptid) (GET_LWP (ptid) != 0) > -#define BUILD_LWP(lwp, pid) ptid_build (pid, lwp, 0) > - > /* Attempt to initialize libthread_db. */ > void check_for_thread_db (void); > -- Joel