Re: [PATCH, cleanup] Standardize access to ptid

[Joel Brobecker <brobecker at adacore dot com>]

Hi Luis,

First of all, thank you for doing this!

> -  int tid = TIDGET (ptid);
> +  int tid = ptid_get_lwp (ptid);

ARGH! I am *SO* glad that you've done this change, because I was
very surprised by this change, and thought it was an oversight.
It's only after I saw the same change a second time that I started
looking at the actual definition of this macro and, surprise:

        defs.h:#define TIDGET(PTID) (ptid_get_lwp (PTID))

Double-argh!

> I've been extra careful to convert from the ptid-building macros to
> the ptid_build function since sol-thread.c and aix-thread.c had some
> variations regarding the position of TID/LWP inside the ptid. Please
> double check.

I did as best as I could, given the length of patch, and it looks
pretty good.

> 2013-09-19  Luis Machado  <lgustavo@codesourcery.com>
> 
> 	* aarch64-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	Replace GET_LWP with ptid_get_lwp.
> 	* aix-thread.c (BUILD_THREAD, BUILD_LWP): Remove.
> 	Replace BUILD_THREAD with ptid_build.
> 	Replace BUILD_LWP with ptid_build.
> 	Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* alphabsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* amd64-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* amd64bsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* arm-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	Replace GET_LWP with ptid_get_lwp.
> 	* armnbsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* auxv.c: Likewise.
> 	* breakpoint.c: Likewise.
> 	* common/ptid.c (ptid_is_pid): Call ptid_is_invalid.
> 	(ptid_is_lwp): New function.
> 	(ptid_is_tid): New function.
> 	(ptid_is_invalid): New function.
> 	* common/ptid.h: Update comments for accessors.
> 	(ptid_is_lwp): New prototype.
> 	(ptid_is_tid): New prototype.
> 	(ptid_is_invalid): New prototype.
> 	* defs.h (PIDGET, TIDGET, MERGEPID): Do not define.
> 	* gcore.c: Replace PIDGET with ptid_get_pid.
> 	* gdbthread.h: Likewise.
> 	* gnu-nat.c: Likewise.
> 	* hppa-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* hppabsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* hppanbsd-nat.c: Likewise.
> 	* i386-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* i386bsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* ia64-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	* infcmd.c: Likewise.
> 	* inferior.h: Likewise.
> 	* inflow.c: Likewise.
> 	* infrun.c: Likewise.
> 	* linux-fork.c: Likewise.
> 	* linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace GET_PID with ptid_get_pid.
> 	Replace is_lwp with ptid_is_lwp.
> 	Replace GET_LWP with ptid_get_lwp.
> 	Replace BUILD_LWP with ptid_build.
> 	* linux-nat.h (GET_LWP, GET_PID, is_lwp, BUILD_LWP): Remove.
> 	* linux-thread-db.c: Replace GET_PID with ptid_get_pid.
> 	Replace GET_LWP with ptid_get_lwp.
> 	Replace BUILD_LWP with ptid_build.
> 	* m32r-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* m68kbsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* m68klinux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* m88kbsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* mi/mi-interp.c: Likewise.
> 	* mi/mi-main.c: Likewise.
> 	* mips64obsd-nat.c: Likewise.
> 	* mipsnbsd-nat.c: Likewise.
> 	* nto-procfs.c: Likewise.
> 	* ppc-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* ppcfbsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* ppcnbsd-nat.c: Likewise.
> 	* ppcobsd-nat.c: Likewise.
> 	* proc-service.c (BUILD_LWP): Remove.
> 	Replace BUILD_LWP with ptid_build.
> 	* procfs.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	Replace MERGEPID with ptid_build.
> 	* python/py-inferior.c: Replace PIDGET with ptid_get_pid.
> 	* python/py-infthread.c: Likewise.
> 	* record.c: Likewise.
> 	* rs6000-nat.c: Likewise.
> 	* s390-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* shnbsd-nat.c: Replace PIDGET with ptid_get_pid.
> 	* sol-thread.c (GET_PID, GET_LWP, GET_THREAD): Remove.
> 	(is_lwp, is_thread, BUILD_LWP, BUILD_THREAD): Remove.
> 	Replace GET_PID and PIDGET with ptid_get_pid.
> 	Replace GET_LWP with ptid_get_lwp.
> 	Replace GET_THREAD with ptid_get_tid.
> 	Replace is_lwp with ptid_is_lwp.
> 	Replace is_thread with ptid_is_tid.
> 	Replace BUILD_LWP and BUILD_THREAD with ptid_build.
> 	* solib-som.c: Replace PIDGET with ptid_get_pid.
> 	* sparc-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.
> 	* spu-linux-nat.c: Likewise.
> 	* target.c: Replace PIDGET with ptid_get_pid.
> 	* thread.c: Likewise.
> 	* vax-nat.c: Likewise.
> 	* vaxbsd-nat.c: Likewise.
> 	* windows-nat.c: Likewise.
> 	* xtensa-linux-nat.c: Replace PIDGET with ptid_get_pid.
> 	Replace TIDGET with ptid_get_lwp.

I only found a couple of issues. The patch looks good to me otherwise,
and is OK to commit once the two trivial issues are fixed.

> @@ -387,7 +387,8 @@ linux_child_follow_fork (struct target_ops *ops, int follow_child,
>    parent_pid = ptid_get_lwp (inferior_ptid);
>    if (parent_pid == 0)
>      parent_pid = ptid_get_pid (inferior_ptid);
> -  child_pid = PIDGET (inferior_thread ()->pending_follow.value.related_pid);
> +  child_pid =
> +    ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);

I think Pedro told me that the '=' binary operator should be on
the second line.

> @@ -4445,7 +4449,7 @@ procfs_init_inferior (struct target_ops *ops, int pid)
>       this point, but it didn't have any lwp info yet.  Notify the core
>       about it.  This changes inferior_ptid as well.  */
>    thread_change_ptid (pid_to_ptid (pid),
> -		      MERGEPID (pid, lwpid));
> +		      ptid_build (pid, lwpid), 0);

I think the first ')' is at the wrong place? Ie...

   		      ptid_build (pid, lwpid, 0));

... instead?

-- 
Joel