[patch] Fix linux-nat.c {,lp->}status typo

[patch] Fix linux-nat.c {,lp->}status typo

[Jan Kratochvil]

Hi,

while reading the code I found this suspicious code, IMO a typo.

              if (WIFSTOPPED (lp->status))
                {
                  if (WSTOPSIG (lp->status) != SIGSTOP)
		  [...]
                }
              else if (WIFEXITED (status) || WIFSIGNALED (status))
	      [...]

STATUS value is not filtered by linux_nat_filter_event.  LP->STATUS is.

That "status = 0;" would be better as some __deinitiailize_var (status) but
there is no such GCC extension.

The DEBUG_LINUX_NAT print could be possibly still STATUS, it depends on the
opinion IMO.

No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.
(I haven't tried to find an impact of this bug.)


Thanks,
Jan


2010-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-nat.c (linux_nat_wait_1): Reset STATUS after calling
	linux_nat_wait_1.  Use always LP->STATUS afterwards.

--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -3365,6 +3365,9 @@ retry:
 
 	  lp = linux_nat_filter_event (lwpid, status, options);
 
+	  /* STATUS is now no longer valid, use LP->STATUS instead.  */
+	  status = 0;
+
 	  if (lp
 	      && ptid_is_pid (ptid)
 	      && ptid_get_pid (lp->ptid) != ptid_get_pid (ptid))
@@ -3373,7 +3376,7 @@ retry:
 
 	      if (debug_linux_nat)
 		fprintf (stderr, "LWP %ld got an event %06x, leaving pending.\n",
-			 ptid_get_lwp (lp->ptid), status);
+			 ptid_get_lwp (lp->ptid), lp->status);
 
 	      if (WIFSTOPPED (lp->status))
 		{
@@ -3410,7 +3413,7 @@ retry:
 		      lp->signalled = 0;
 		    }
 		}
-	      else if (WIFEXITED (status) || WIFSIGNALED (status))
+	      else if (WIFEXITED (lp->status) || WIFSIGNALED (lp->status))
 		{
 		  if (debug_linux_nat)
 		    fprintf (stderr, "Process %ld exited while stopping LWPs\n",

Re: [patch] Fix linux-nat.c {,lp->}status typo

[Pedro Alves]

On Sunday 18 July 2010 15:51:12, Jan Kratochvil wrote:

> while reading the code I found this suspicious code, IMO a typo.
> 
>               if (WIFSTOPPED (lp->status))
>                 {
>                   if (WSTOPSIG (lp->status) != SIGSTOP)
> 		  [...]
>                 }
>               else if (WIFEXITED (status) || WIFSIGNALED (status))
> 	      [...]
> 

> No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu.
> (I haven't tried to find an impact of this bug.)

In practice, status == lp->status at this point for !WIFSTOPPED,
so there should be no impact.

> 2010-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* linux-nat.c (linux_nat_wait_1): Reset STATUS after calling
> 	linux_nat_wait_1.  Use always LP->STATUS afterwards.

Okay, thanks.

-- 
Pedro Alves

Re: [patch] Fix linux-nat.c {,lp->}status typo

[Jan Kratochvil]

On Tue, 20 Jul 2010 14:22:28 +0200, Pedro Alves wrote:
> On Sunday 18 July 2010 15:51:12, Jan Kratochvil wrote:
> In practice, status == lp->status at this point for !WIFSTOPPED,
> so there should be no impact.

You are right as I see now.  OK, marked ChangeLog entry only as "Code cleanup".


> Okay, thanks.

Checked-in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-07/msg00112.html

--- src/gdb/ChangeLog	2010/07/20 05:58:52	1.12000
+++ src/gdb/ChangeLog	2010/07/20 12:33:29	1.12001
@@ -1,3 +1,9 @@
+2010-07-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
+	Code cleanup.
+	* linux-nat.c (linux_nat_wait_1): Reset STATUS after calling
+	linux_nat_wait_1.  Use always LP->STATUS afterwards.
+
 2010-07-20  Hui Zhu  <teawater@gmail.com>
 
 	* inf-ptrace.c (inf_ptrace_create_inferior): Initialize back_to.
--- src/gdb/linux-nat.c	2010/07/18 21:21:53	1.174
+++ src/gdb/linux-nat.c	2010/07/20 12:33:30	1.175
@@ -3367,6 +3367,9 @@
 
 	  lp = linux_nat_filter_event (lwpid, status, options);
 
+	  /* STATUS is now no longer valid, use LP->STATUS instead.  */
+	  status = 0;
+
 	  if (lp
 	      && ptid_is_pid (ptid)
 	      && ptid_get_pid (lp->ptid) != ptid_get_pid (ptid))
@@ -3375,7 +3378,7 @@
 
 	      if (debug_linux_nat)
 		fprintf (stderr, "LWP %ld got an event %06x, leaving pending.\n",
-			 ptid_get_lwp (lp->ptid), status);
+			 ptid_get_lwp (lp->ptid), lp->status);
 
 	      if (WIFSTOPPED (lp->status))
 		{
@@ -3412,7 +3415,7 @@
 		      lp->signalled = 0;
 		    }
 		}
-	      else if (WIFEXITED (status) || WIFSIGNALED (status))
+	      else if (WIFEXITED (lp->status) || WIFSIGNALED (lp->status))
 		{
 		  if (debug_linux_nat)
 		    fprintf (stderr, "Process %ld exited while stopping LWPs\n",