[PATCH] Fix argument-passing error in linux-nat.c

[PATCH] Fix argument-passing error in linux-nat.c

[Sérgio Durigan Júnior]

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

Hi guys,

This is a very trivial patch that I have done (while I don't have yet
the 'catch syscall' patch! :D). Well, I found this little error in
linux-nat.c. Basically, the 'status' variable in the function
'get_pending_status' is a pointer to an integer, but the macros from
waitpid.h (WIFSTOPPED and WSTOPSIG) are being called with this pointer
as the argument, instead of the integer itself. It was just a matter of
putting a '*' in the front of the name ;-)

This fix did not cause any improvement on the testsuite (for 32-bit PPC
at least), but anyway it's better to do things right, huh? Also, as I'm
sending this e-mail, I'd like to propose one little modification in the
GDB code. As you all know GCC does not do type-checking in macros, which
makes it hard to identify problems like this one. To avoid future
issues, it would be a good thing to make a "wrapper" for these macros
(specially for those in waitpid.h). This wrapper could be something like
a "static inline", so we won't miss anything in the performance aspect.
For example, in the case of WSTOPSIG:

static inline int
internal_WSTOPSIG (int status)
{
  return WSTOPSIG (status);
}

So, whenever we call internal_WSTOPSIG (ugly name, we certainly may want
to choose another one), we have *for free* a type-checking for its
argument :-).

I took a quick look at the code and there are not so many places to
replace the calls, so I think it would not be a hard job to do...

So, what do you think?

Thanks,

-- 
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil

[-- Attachment #2: linux-nat-status-pointer.patch --]
[-- Type: text/x-patch, Size: 559 bytes --]

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 42e19b5..0851492 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1442,8 +1442,8 @@ get_pending_status (struct lwp_info *lp, int *status)
 	     queue.  */
 	  if (queued_waitpid (GET_LWP (lp->ptid), status, __WALL) != -1)
 	    {
-	      if (WIFSTOPPED (status))
-		signo = target_signal_from_host (WSTOPSIG (status));
+	      if (WIFSTOPPED (*status))
+		signo = target_signal_from_host (WSTOPSIG (*status));
 
 	      /* If not stopped, then the lwp is gone, no use in
 		 resending a signal.  */

Re: [PATCH] Fix argument-passing error in linux-nat.c

[Pedro Alves]

On Thursday 25 September 2008 14:15:23, Sérgio Durigan Júnior wrote:
> This is a very trivial patch that I have done (while I don't have yet
> the 'catch syscall' patch! :D). Well, I found this little error in
> linux-nat.c. Basically, the 'status' variable in the function
> 'get_pending_status' is a pointer to an integer, but the macros from
> waitpid.h (WIFSTOPPED and WSTOPSIG) are being called with this pointer
> as the argument, instead of the integer itself. It was just a matter of
> putting a '*' in the front of the name ;-)

Well, I'll be ...  Your change is obviously correct.  Can you check it in,
with a ChangeLog entry?

How many typos can one write. :-)  Sorry 'bout that.

> This fix did not cause any improvement on the testsuite (for 32-bit PPC
> at least), but anyway it's better to do things right, huh? 

You can only trigger this code path in non-stop mode (set
target-async + set non-stop 1), and we don't have any non-stop test 
attach/detaching.

-- 
Pedro Alves

Re: [PATCH] Fix argument-passing error in linux-nat.c

[Thiago Jung Bauermann]

El jue, 25-09-2008 a las 14:52 +0100, Pedro Alves escribió:
> On Thursday 25 September 2008 14:15:23, Sérgio Durigan Júnior wrote:
> Well, I'll be ...  Your change is obviously correct.  Can you check it in,
> with a ChangeLog entry?

Committed for him, with ChangeLog entry. Thanks for the quick review.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center