[PATCH] Move code to common/ptid.h

[PATCH] Move code to common/ptid.h

[Luis Machado]

[-- Attachment #1: Type: text/plain, Size: 162 bytes --]

Hi,

One more piece of cleanup. These macros currently live in linux-nat.h, 
but they should really go to common/ptid.h so gdbserver can use them as 
well.

OK?


[-- Attachment #2: ptid.diff --]
[-- Type: text/x-patch, Size: 1452 bytes --]

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.

 gdb/common/ptid.h | 6 ++++++
 gdb/linux-nat.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);
 

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

[Pedro Alves]

On 09/19/2013 01:16 PM, Luis Machado wrote:
> Hi,
> 
> One more piece of cleanup. These macros currently live in linux-nat.h, 
> but they should really go to common/ptid.h so gdbserver can use them as 
> well.
> 
> OK?
> 

NAK.  These are old macros that just add another way to do
the same thing.  We don't want to promote that in core code.

-- 
Pedro Alves

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

[Joel Brobecker]

> 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

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

[Luis Machado]

On 09/19/2013 09:34 AM, Joel Brobecker wrote:
>> 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.
>

Thanks Joel. Well, i feel the same, but this is old cruft that should 
either get out of the way or get out of the code.

As is, these are just useless constructions that keep getting in the way 
of duplication removal.

So, let's do it differently. Let's get rid of all these macros and their 
uses. How does that sound?

Luis

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

[Pedro Alves]

On 09/19/2013 01:44 PM, Luis Machado wrote:

> Thanks Joel. Well, i feel the same, but this is old cruft that should 
> either get out of the way or get out of the code.
> 
> As is, these are just useless constructions that keep getting in the way 
> of duplication removal.
> 
> So, let's do it differently. Let's get rid of all these macros and their 
> uses. How does that sound?

+1

-- 
Pedro Alves