New ARI warning Wed Apr 27 01:54:55 UTC 2011

New ARI warning Wed Apr 27 01:54:55 UTC 2011

[GDB Administrator]

119a120
> gdb/common/linux-ptrace.h:22: regression: wait.h: Do not include wait.h or sys/wait.h, instead include gdb_wait.h
gdb/common/linux-ptrace.h:22:#include <sys/wait.h>  

Re: New ARI warning Wed Apr 27 01:54:55 UTC 2011

[Yao Qi]

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

On 04/27/2011 09:54 AM, GDB Administrator wrote:
> 119a120
>> gdb/common/linux-ptrace.h:22: regression: wait.h: Do not include wait.h or sys/wait.h, instead include gdb_wait.h
> gdb/common/linux-ptrace.h:22:#include <sys/wait.h>  

sys/wait.h is included for macro __WALL.  However, we can safely remove
this include from common/linux-ptrace.h because either sys/wait.h or
gdb_wait.h is included before including linux-ptrace.h in linux-nat.c
and linux-low.c.

In linux-nat.c, gdb_wait.h is included in line 25, and linux-ptrace.h is
included in line 33.  In linux-low.c, sys/wait.h is included in line 23,
and linux-ptrace.h is included in line 27.

Is it OK?

-- 
Yao (齐尧)

[-- Attachment #2: remove_include_sys_wait.patch --]
[-- Type: text/x-patch, Size: 587 bytes --]

2011-04-27  Yao Qi  <yao@codesourcery.com>

	* common/linux-ptrace.h: Remove include <sys/wait.h>.

Index: common/linux-ptrace.h
===================================================================
RCS file: /cvs/src/src/gdb/common/linux-ptrace.h,v
retrieving revision 1.1
diff -u -r1.1 linux-ptrace.h
--- common/linux-ptrace.h	26 Apr 2011 15:36:04 -0000	1.1
+++ common/linux-ptrace.h	27 Apr 2011 02:52:53 -0000
@@ -19,7 +19,6 @@
 #define COMMON_LINUX_PTRACE_H
 
 #include <sys/ptrace.h>
-#include <sys/wait.h> /* __WAIT */
 
 #ifndef PTRACE_GETSIGINFO
 # define PTRACE_GETSIGINFO 0x4202

Re: New ARI warning Wed Apr 27 01:54:55 UTC 2011

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2011-04-27  Yao Qi  <yao@codesourcery.com>
Yao> 	* common/linux-ptrace.h: Remove include <sys/wait.h>.

Ok.

Tom

Re: New ARI warning Wed Apr 27 01:54:55 UTC 2011

[Joel Brobecker]

> sys/wait.h is included for macro __WALL.  However, we can safely remove
> this include from common/linux-ptrace.h because either sys/wait.h or
> gdb_wait.h is included before including linux-ptrace.h in linux-nat.c
> and linux-low.c.

That's the only pragmatic answer that we have right now, but I do
think that this is an extremely bad practice. Or maybe I'm biased
by my past as an Ada developer...

Sooner or later, we'll have to move these header files to the common
area as well.

-- 
Joel

Re: New ARI warning Wed Apr 27 01:54:55 UTC 2011

[Yao Qi]

On 04/27/2011 11:08 PM, Joel Brobecker wrote:
>> sys/wait.h is included for macro __WALL.  However, we can safely remove
>> this include from common/linux-ptrace.h because either sys/wait.h or
>> gdb_wait.h is included before including linux-ptrace.h in linux-nat.c
>> and linux-low.c.
> 
> That's the only pragmatic answer that we have right now, but I do
> think that this is an extremely bad practice. Or maybe I'm biased
> by my past as an Ada developer...
> 
> Sooner or later, we'll have to move these header files to the common
> area as well.
> 

Yes, I agree.  So far, gdb code is using gdb_wait.h and gdbserver is
using sys/wait.h.  gdb_wait.h looks quite independent of gdb or
gdbserver.  Is there any known reason that we can't use gdb_wait.h in
gdbserver?  I don't see any.

-- 
Yao (齐尧)

Re: New ARI warning Wed Apr 27 01:54:55 UTC 2011

[Joel Brobecker]

> Yes, I agree.  So far, gdb code is using gdb_wait.h and gdbserver is
> using sys/wait.h.  gdb_wait.h looks quite independent of gdb or
> gdbserver.  Is there any known reason that we can't use gdb_wait.h in
> gdbserver?  I don't see any.

You need to make sure that both configures test for sys/wait.h and
wait.h. I looked at gdbserver's configure, and it is missing the check
for wait.

-- 
Joel

Re: New ARI warning Wed Apr 27 01:54:55 UTC 2011

[Mark Kettenis]

> X-SWARE-Spam-Status: No, hits=-2.0 required=5.0	tests=AWL,BAYES_00
> X-Spam-Check-By: sourceware.org
> Date: Wed, 27 Apr 2011 09:18:02 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> Content-Disposition: inline
> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
> Sender: gdb-patches-owner@sourceware.org
> X-XS4ALL-DNSBL-Checked: mxdrop220.xs4all.nl checked 209.132.180.131 against DNS blacklists
> X-CNFS-Analysis: v=1.1 cv=a8sYchbnJd2dMYUqqjolUMD0rF/qLqJCTuzWyWz0xZo= c=1
> 	sm=0 a=XYJHFtupD_QA:10 a=msjzrvmC9sEA:10 a=idPYu1UqwmkA:10
> 	a=wPDyFdB5xvgA:10 a=kj9zAlcOel0A:10 a=vbYRN7G9ZuyAWxq09MFwFw==:17
> 	a=s8b-tOObir8ccsWKurEA:9 a=CjuIK1q_8ugA:10
> 	a=vbYRN7G9ZuyAWxq09MFwFw==:117
> X-Virus-Scanned: by XS4ALL Virus Scanner
> X-XS4ALL-Spam-Score: -0.0 () SPF_HELO_PASS, SPF_PASS
> X-XS4ALL-Spam: NO
> Envelope-To: m.m.kettenis@xs4all.nl
> 
> > Yes, I agree.  So far, gdb code is using gdb_wait.h and gdbserver is
> > using sys/wait.h.  gdb_wait.h looks quite independent of gdb or
> > gdbserver.  Is there any known reason that we can't use gdb_wait.h in
> > gdbserver?  I don't see any.
> 
> You need to make sure that both configures test for sys/wait.h and
> wait.h. I looked at gdbserver's configure, and it is missing the check
> for wait.

And suddenly even that "trivial" linux-ptrace.h diff is turning into a
can of worms.  Some people, including me, have stated that the stuff in
common/ should *not* depend on any configure checks

In this case there probably is a way out though.  I don't think there
are any systems out there that don't have <sys/wait.h>.  So we could
just get rid of gdb_wait.h altogether.

But this doesn't encourage me to give my blessing to the i386-dbg diff...

Re: New ARI warning Wed Apr 27 01:54:55 UTC 2011

[Joel Brobecker]

> And suddenly even that "trivial" linux-ptrace.h diff is turning into a
> can of worms.  Some people, including me, have stated that the stuff in
> common/ should *not* depend on any configure checks

Do you think that's attainable, though? There are a lot of basic types
which depend on configure... There might be some code such as gdb_select
which might be useful to gdbserver as well, one day.  I'm sure we might
be able to implement some of the code without the use of configure, but
configure helps keep things cleaner.

> In this case there probably is a way out though.  I don't think there
> are any systems out there that don't have <sys/wait.h>.  So we could
> just get rid of gdb_wait.h altogether.

Is that because sys/wait is somehow mandated (by C90, perhaps?).
I'm wondering why we had it in the first place, it must have been
useful at some point...

-- 
Joel