Re: [PATCH] Move code to common/ptid.h

[Joel Brobecker <brobecker at adacore dot com>]

> 2013-09-19  Luis Machado  <lgustavo@codesourcery.com>
> 
> 	* 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