[common] Merge duplicated macros in linux-nat.c and linux-low.c

[common] Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

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

Some ptrace related macros are duplicated in linux-nat.c and
gdbserver/linux-low.c.  This patch is to move them together in
common/linux-common.h.

Rebuild gdb and gdbserver on x86-linux successfully.  Note that the
dependency tracking in gdbserver is not done in this patch.

-- 
Yao (齐尧)

[-- Attachment #2: linux-common-h-0218.patch --]
[-- Type: text/x-patch, Size: 6539 bytes --]

gdb/

	* linux-nat.c: Move common macros to ...
	* common/linux-common.h: ... here.  New.
	* linux-nat.h: Include linux-common.h.

gdb/gdbserver/

	* linux-low.c: Move common macros to linux-common.h.
	* linux-low.h: Include linux-common.h.


diff --git a/gdb/common/linux-common.h b/gdb/common/linux-common.h
new file mode 100644
index 0000000..8d54d51
--- /dev/null
+++ b/gdb/common/linux-common.h
@@ -0,0 +1,60 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/ptrace.h>
+
+#ifndef PTRACE_GETSIGINFO
+# define PTRACE_GETSIGINFO 0x4202
+# define PTRACE_SETSIGINFO 0x4203
+#endif
+
+/* If the system headers did not provide the constants, hard-code the normal
+   values.  */
+#ifndef PTRACE_EVENT_FORK
+
+#define PTRACE_SETOPTIONS	0x4200
+#define PTRACE_GETEVENTMSG	0x4201
+
+/* options set using PTRACE_SETOPTIONS */
+#define PTRACE_O_TRACESYSGOOD	0x00000001
+#define PTRACE_O_TRACEFORK	0x00000002
+#define PTRACE_O_TRACEVFORK	0x00000004
+#define PTRACE_O_TRACECLONE	0x00000008
+#define PTRACE_O_TRACEEXEC	0x00000010
+#define PTRACE_O_TRACEVFORKDONE	0x00000020
+#define PTRACE_O_TRACEEXIT	0x00000040
+
+/* Wait extended result codes for the above trace options.  */
+#define PTRACE_EVENT_FORK	1
+#define PTRACE_EVENT_VFORK	2
+#define PTRACE_EVENT_CLONE	3
+#define PTRACE_EVENT_EXEC	4
+#define PTRACE_EVENT_VFORK_DONE	5
+#define PTRACE_EVENT_EXIT	6
+
+#endif /* PTRACE_EVENT_FORK */
+
+#ifndef O_LARGEFILE
+#define O_LARGEFILE 0
+#endif
+
+/* We can't always assume that this flag is available, but all systems
+   with the ptrace event handlers also have __WALL, so it's safe to use
+   in some contexts.  */
+#ifndef __WALL
+#define __WALL          0x40000000 /* Wait for any child.  */
+#endif
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 93f3570..1d9d150 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -52,47 +52,9 @@
 #define SPUFS_MAGIC 0x23c9b64e
 #endif
 
-#ifndef PTRACE_GETSIGINFO
-# define PTRACE_GETSIGINFO 0x4202
-# define PTRACE_SETSIGINFO 0x4203
-#endif
 
-#ifndef O_LARGEFILE
-#define O_LARGEFILE 0
-#endif
 
-/* If the system headers did not provide the constants, hard-code the normal
-   values.  */
-#ifndef PTRACE_EVENT_FORK
-
-#define PTRACE_SETOPTIONS	0x4200
-#define PTRACE_GETEVENTMSG	0x4201
-
-/* options set using PTRACE_SETOPTIONS */
-#define PTRACE_O_TRACESYSGOOD	0x00000001
-#define PTRACE_O_TRACEFORK	0x00000002
-#define PTRACE_O_TRACEVFORK	0x00000004
-#define PTRACE_O_TRACECLONE	0x00000008
-#define PTRACE_O_TRACEEXEC	0x00000010
-#define PTRACE_O_TRACEVFORKDONE	0x00000020
-#define PTRACE_O_TRACEEXIT	0x00000040
-
-/* Wait extended result codes for the above trace options.  */
-#define PTRACE_EVENT_FORK	1
-#define PTRACE_EVENT_VFORK	2
-#define PTRACE_EVENT_CLONE	3
-#define PTRACE_EVENT_EXEC	4
-#define PTRACE_EVENT_VFORK_DONE	5
-#define PTRACE_EVENT_EXIT	6
-
-#endif /* PTRACE_EVENT_FORK */
-
-/* We can't always assume that this flag is available, but all systems
-   with the ptrace event handlers also have __WALL, so it's safe to use
-   in some contexts.  */
-#ifndef __WALL
-#define __WALL          0x40000000 /* Wait for any child.  */
-#endif
+
 
 #ifndef W_STOPCODE
 #define W_STOPCODE(sig) ((sig) << 8 | 0x7f)
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 6635bc6..0891e06 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -22,6 +22,7 @@
 #endif
 
 #include "gdb_proc_service.h"
+#include "linux-common.h"
 
 #ifdef HAVE_LINUX_REGSETS
 typedef void (*regset_fill_func) (struct regcache *, void *);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index cd88df8..b6c78cd 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -156,53 +156,12 @@ But we would still have to have some support for SIGSTOP, since PTRACE_ATTACH
 generates it, and there are races with trying to find a signal that is not
 blocked.  */
 
-#ifndef O_LARGEFILE
-#define O_LARGEFILE 0
-#endif
-
-/* If the system headers did not provide the constants, hard-code the normal
-   values.  */
-#ifndef PTRACE_EVENT_FORK
-
-#define PTRACE_SETOPTIONS	0x4200
-#define PTRACE_GETEVENTMSG	0x4201
-
-/* Options set using PTRACE_SETOPTIONS.  */
-#define PTRACE_O_TRACESYSGOOD	0x00000001
-#define PTRACE_O_TRACEFORK	0x00000002
-#define PTRACE_O_TRACEVFORK	0x00000004
-#define PTRACE_O_TRACECLONE	0x00000008
-#define PTRACE_O_TRACEEXEC	0x00000010
-#define PTRACE_O_TRACEVFORKDONE	0x00000020
-#define PTRACE_O_TRACEEXIT	0x00000040
-
-/* Wait extended result codes for the above trace options.  */
-#define PTRACE_EVENT_FORK	1
-#define PTRACE_EVENT_VFORK	2
-#define PTRACE_EVENT_CLONE	3
-#define PTRACE_EVENT_EXEC	4
-#define PTRACE_EVENT_VFORK_DONE	5
-#define PTRACE_EVENT_EXIT	6
-
-#endif /* PTRACE_EVENT_FORK */
 
 /* Unlike other extended result codes, WSTOPSIG (status) on
    PTRACE_O_TRACESYSGOOD syscall events doesn't return SIGTRAP, but
    instead SIGTRAP with bit 7 set.  */
 #define SYSCALL_SIGTRAP (SIGTRAP | 0x80)
 
-/* We can't always assume that this flag is available, but all systems
-   with the ptrace event handlers also have __WALL, so it's safe to use
-   here.  */
-#ifndef __WALL
-#define __WALL          0x40000000 /* Wait for any child.  */
-#endif
-
-#ifndef PTRACE_GETSIGINFO
-# define PTRACE_GETSIGINFO    0x4202
-# define PTRACE_SETSIGINFO    0x4203
-#endif
-
 /* The single-threaded native GNU/Linux target_ops.  We save a pointer for
    the use of the multi-threaded target.  */
 static struct target_ops *linux_ops;
diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
index 42cb2fc..cd4bf5d 100644
--- a/gdb/linux-nat.h
+++ b/gdb/linux-nat.h
@@ -19,7 +19,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "target.h"
-
+#include "linux-common.h"
 #include <signal.h>
 
 /* Structure describing an LWP.  This is public only for the purposes

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Jan Kratochvil]

Hi Yao,

On Fri, 18 Feb 2011 08:30:41 +0100, Yao Qi wrote:
> diff --git a/gdb/common/linux-common.h b/gdb/common/linux-common.h
> new file mode 100644
> index 0000000..8d54d51
> --- /dev/null
> +++ b/gdb/common/linux-common.h
> @@ -0,0 +1,60 @@
> +/* Copyright (C) 2011 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/ptrace.h>
> +
> +#ifndef PTRACE_GETSIGINFO
> +# define PTRACE_GETSIGINFO 0x4202
> +# define PTRACE_SETSIGINFO 0x4203
> +#endif

There should be a wrapper:

#ifndef COMMON_LINUX_COMMON_H
#define COMMON_LINUX_COMMON_H
#endif /* COMMON_LINUX_COMMON_H */



> +/* options set using PTRACE_SETOPTIONS */

[nitpick] This comment formatting was better in linux-nat.c


> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
> index 6635bc6..0891e06 100644
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -22,6 +22,7 @@
>  #endif
>  
>  #include "gdb_proc_service.h"
> +#include "linux-common.h"

If you moved the definitions out of linux-low.c it is enough
to #include "linux-common.h" also just in linux-low.c.


> --- a/gdb/linux-nat.h
> +++ b/gdb/linux-nat.h
> @@ -19,7 +19,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "target.h"
> -
> +#include "linux-common.h"
>  #include <signal.h>
>  
>  /* Structure describing an LWP.  This is public only for the purposes

If you moved the definitions out of linux-nat.c it is enough
to #include "linux-common.h" also just in linux-nat.c.


It is fine with me with these changes although Pedro may want to approve the
gdbserver/ part.


Thanks,
Jan

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

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

On 02/18/2011 04:24 PM, Jan Kratochvil wrote:

Jan,
Thanks for the quick review.

> There should be a wrapper:
> 
> #ifndef COMMON_LINUX_COMMON_H
> #define COMMON_LINUX_COMMON_H
> #endif /* COMMON_LINUX_COMMON_H */
> 

OK.  Done.

>> >  #include "gdb_proc_service.h"
>> > +#include "linux-common.h"
> If you moved the definitions out of linux-low.c it is enough
> to #include "linux-common.h" also just in linux-low.c.
> 

Oh, yes.  Fixed.

> If you moved the definitions out of linux-nat.c it is enough
> to #include "linux-common.h" also just in linux-nat.c.

Fixed as well.

-- 
Yao (齐尧)

[-- Attachment #2: linux-common-h-0218-r2.patch --]
[-- Type: text/x-patch, Size: 6317 bytes --]

gdb/

        * linux-nat.c: Move common macros to ...
        Include linux-common.h.
        * common/linux-common.h: ... here.  New.

gdb/gdbserver/

        * linux-low.c: Move common macros to linux-common.h.
        Include linux-common.h.

diff --git a/gdb/common/linux-common.h b/gdb/common/linux-common.h
new file mode 100644
index 0000000..1987ea5
--- /dev/null
+++ b/gdb/common/linux-common.h
@@ -0,0 +1,65 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef COMMON_LINUX_COMMON_H
+#define COMMON_LINUX_COMMON_H
+
+#include <sys/ptrace.h>
+
+#ifndef PTRACE_GETSIGINFO
+# define PTRACE_GETSIGINFO 0x4202
+# define PTRACE_SETSIGINFO 0x4203
+#endif /* PTRACE_GETSIGINF */
+
+/* If the system headers did not provide the constants, hard-code the normal
+   values.  */
+#ifndef PTRACE_EVENT_FORK
+
+#define PTRACE_SETOPTIONS	0x4200
+#define PTRACE_GETEVENTMSG	0x4201
+
+/* options set using PTRACE_SETOPTIONS */
+#define PTRACE_O_TRACESYSGOOD	0x00000001
+#define PTRACE_O_TRACEFORK	0x00000002
+#define PTRACE_O_TRACEVFORK	0x00000004
+#define PTRACE_O_TRACECLONE	0x00000008
+#define PTRACE_O_TRACEEXEC	0x00000010
+#define PTRACE_O_TRACEVFORKDONE	0x00000020
+#define PTRACE_O_TRACEEXIT	0x00000040
+
+/* Wait extended result codes for the above trace options.  */
+#define PTRACE_EVENT_FORK	1
+#define PTRACE_EVENT_VFORK	2
+#define PTRACE_EVENT_CLONE	3
+#define PTRACE_EVENT_EXEC	4
+#define PTRACE_EVENT_VFORK_DONE	5
+#define PTRACE_EVENT_EXIT	6
+
+#endif /* PTRACE_EVENT_FORK */
+
+#ifndef O_LARGEFILE
+#define O_LARGEFILE 0
+#endif
+
+/* We can't always assume that this flag is available, but all systems
+   with the ptrace event handlers also have __WALL, so it's safe to use
+   in some contexts.  */
+#ifndef __WALL
+#define __WALL          0x40000000 /* Wait for any child.  */
+#endif
+
+#endif /* COMMON_LINUX_COMMON_H */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 93f3570..4b16366 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -24,6 +24,7 @@
 #include <stdio.h>
 #include <sys/param.h>
 #include <sys/ptrace.h>
+#include "linux-common.h"
 #include <signal.h>
 #include <sys/ioctl.h>
 #include <fcntl.h>
@@ -52,47 +53,9 @@
 #define SPUFS_MAGIC 0x23c9b64e
 #endif
 
-#ifndef PTRACE_GETSIGINFO
-# define PTRACE_GETSIGINFO 0x4202
-# define PTRACE_SETSIGINFO 0x4203
-#endif
 
-#ifndef O_LARGEFILE
-#define O_LARGEFILE 0
-#endif
 
-/* If the system headers did not provide the constants, hard-code the normal
-   values.  */
-#ifndef PTRACE_EVENT_FORK
-
-#define PTRACE_SETOPTIONS	0x4200
-#define PTRACE_GETEVENTMSG	0x4201
-
-/* options set using PTRACE_SETOPTIONS */
-#define PTRACE_O_TRACESYSGOOD	0x00000001
-#define PTRACE_O_TRACEFORK	0x00000002
-#define PTRACE_O_TRACEVFORK	0x00000004
-#define PTRACE_O_TRACECLONE	0x00000008
-#define PTRACE_O_TRACEEXEC	0x00000010
-#define PTRACE_O_TRACEVFORKDONE	0x00000020
-#define PTRACE_O_TRACEEXIT	0x00000040
-
-/* Wait extended result codes for the above trace options.  */
-#define PTRACE_EVENT_FORK	1
-#define PTRACE_EVENT_VFORK	2
-#define PTRACE_EVENT_CLONE	3
-#define PTRACE_EVENT_EXEC	4
-#define PTRACE_EVENT_VFORK_DONE	5
-#define PTRACE_EVENT_EXIT	6
-
-#endif /* PTRACE_EVENT_FORK */
-
-/* We can't always assume that this flag is available, but all systems
-   with the ptrace event handlers also have __WALL, so it's safe to use
-   in some contexts.  */
-#ifndef __WALL
-#define __WALL          0x40000000 /* Wait for any child.  */
-#endif
+
 
 #ifndef W_STOPCODE
 #define W_STOPCODE(sig) ((sig) << 8 | 0x7f)
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index cd88df8..c4d61fc 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -30,6 +30,7 @@
 #endif
 #include <sys/ptrace.h>
 #include "linux-nat.h"
+#include "linux-common.h"
 #include "linux-fork.h"
 #include "gdbthread.h"
 #include "gdbcmd.h"
@@ -156,53 +157,12 @@ But we would still have to have some support for SIGSTOP, since PTRACE_ATTACH
 generates it, and there are races with trying to find a signal that is not
 blocked.  */
 
-#ifndef O_LARGEFILE
-#define O_LARGEFILE 0
-#endif
-
-/* If the system headers did not provide the constants, hard-code the normal
-   values.  */
-#ifndef PTRACE_EVENT_FORK
-
-#define PTRACE_SETOPTIONS	0x4200
-#define PTRACE_GETEVENTMSG	0x4201
-
-/* Options set using PTRACE_SETOPTIONS.  */
-#define PTRACE_O_TRACESYSGOOD	0x00000001
-#define PTRACE_O_TRACEFORK	0x00000002
-#define PTRACE_O_TRACEVFORK	0x00000004
-#define PTRACE_O_TRACECLONE	0x00000008
-#define PTRACE_O_TRACEEXEC	0x00000010
-#define PTRACE_O_TRACEVFORKDONE	0x00000020
-#define PTRACE_O_TRACEEXIT	0x00000040
-
-/* Wait extended result codes for the above trace options.  */
-#define PTRACE_EVENT_FORK	1
-#define PTRACE_EVENT_VFORK	2
-#define PTRACE_EVENT_CLONE	3
-#define PTRACE_EVENT_EXEC	4
-#define PTRACE_EVENT_VFORK_DONE	5
-#define PTRACE_EVENT_EXIT	6
-
-#endif /* PTRACE_EVENT_FORK */
 
 /* Unlike other extended result codes, WSTOPSIG (status) on
    PTRACE_O_TRACESYSGOOD syscall events doesn't return SIGTRAP, but
    instead SIGTRAP with bit 7 set.  */
 #define SYSCALL_SIGTRAP (SIGTRAP | 0x80)
 
-/* We can't always assume that this flag is available, but all systems
-   with the ptrace event handlers also have __WALL, so it's safe to use
-   here.  */
-#ifndef __WALL
-#define __WALL          0x40000000 /* Wait for any child.  */
-#endif
-
-#ifndef PTRACE_GETSIGINFO
-# define PTRACE_GETSIGINFO    0x4202
-# define PTRACE_SETSIGINFO    0x4203
-#endif
-
 /* The single-threaded native GNU/Linux target_ops.  We save a pointer for
    the use of the multi-threaded target.  */
 static struct target_ops *linux_ops;

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Mark Kettenis]

> Date: Fri, 18 Feb 2011 22:41:26 +0800
> From: Yao Qi <yao@codesourcery.com>

> +#include <sys/ptrace.h>
> +

You only include <sys/ptrace.h> in this header file, but...

> +
> +#ifndef O_LARGEFILE
> +#define O_LARGEFILE 0
> +#endif
> +
> +/* We can't always assume that this flag is available, but all systems
> +   with the ptrace event handlers also have __WALL, so it's safe to use
> +   in some contexts.  */
> +#ifndef __WALL
> +#define __WALL          0x40000000 /* Wait for any child.  */
> +#endif

...I don't think O_LARGEFILE and __WALL are declared in that header
file.  I wonder if you should also include <fcntl.h> and <sys/wait.h>
here, to make sure you don't accidentally define them to some bogus
value when the Linux system does provide the proper defines for them.

Cheers,

Mark

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Tom Tromey]

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

Yao> Note that the dependency tracking in gdbserver is not done in this
Yao> patch.

Why not?
It seems like it should be.

Tom

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

On 02/18/2011 11:14 PM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
> 
> Yao> Note that the dependency tracking in gdbserver is not done in this
> Yao> patch.
> 
> Why not?
> It seems like it should be.

I thought once gdbserver is converted to automake, it can be done
easily, so I don't that.

-- 
Yao (齐尧)

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Pedro Alves]

One general comment I have with moving things under common/,
--- just a though towards the whole sharing process
itself; not on this particular patch --- is that I'd very
much like us to avoid putting "common" on the filename of
files we end up putting under common/.  That's just
redundant.  Imagine we end up moving one of these files
elsewhere.  Then we'd need to rename them again.  Or that
we end up with a single implementation of the target
backend (we get rid of all the linux-nat.c code, for
example) --- then "common" on the filename stops making
sense at that point as well.

Let's name the files according to their contents instead.
Let's take the opportunity to split independent things
into separate files if it makes sense to factor them out.

E.g., x86 watchpoint related macros -> x86-watch.h
or x86-nat-watch.h or i386-watchpoint.h, or something
like that, not x86-common.h.

E.g., linux ptrace related macros and definition
  -> linux-ptrace.h or something like that.

I realize that there are some files where "common"
will make most sense, and we shouldn't spend time
bikeshedding on filenames.  This patch may well be
one of those.  Again, I'm speaking to the whole
process, not particularly to this patch.

-- 
Pedro Alves

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Tom Tromey]

Yao> I thought once gdbserver is converted to automake, it can be done
Yao> easily, so I don't that.

Yeah, definitely don't wait for that to happen.
I'm randomly hacking on it but I am not positive I will finish it.
In the meantime the dependencies should be correct.

Tom

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Pedro Alves]

On Friday 18 February 2011 08:24:20, Jan Kratochvil wrote:

> It is fine with me with these changes although Pedro may want to approve the
> gdbserver/ part.

Thanks.  I have nothing else to add.

-- 
Pedro Alves

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

On 02/18/2011 11:46 PM, Pedro Alves wrote:
> Let's name the files according to their contents instead.
> Let's take the opportunity to split independent things
> into separate files if it makes sense to factor them out.
> 

Yes, I agree.  There have been some files named *-common in my local
gdb/common/ dir. :)  They look odd on name.

> E.g., x86 watchpoint related macros -> x86-watch.h
> or x86-nat-watch.h or i386-watchpoint.h, or something
> like that, not x86-common.h.
>
> E.g., linux ptrace related macros and definition
>   -> linux-ptrace.h or something like that.
> 

Looks good to me.  I'll name my files in this way.

-- 
Yao (齐尧)

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

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

On 02/19/2011 12:58 AM, Yao Qi wrote:
>> > E.g., x86 watchpoint related macros -> x86-watch.h
>> > or x86-nat-watch.h or i386-watchpoint.h, or something
>> > like that, not x86-common.h.
>> >
>> > E.g., linux ptrace related macros and definition
>> >   -> linux-ptrace.h or something like that.
>> > 
> Looks good to me.  I'll name my files in this way.

All the comments in previous mails are address in this new patch,
1.  rename 'linux-common.h' to 'linux-ptrace.h', which includes only
ptrace-related macros.
2.  include <sys/wait.h> in linux-ptrace.h for macro __WALL,
3.  add dependency tracking for gdbserver,

-- 
Yao (齐尧)

[-- Attachment #2: linux-ptrace-h-0220.patch --]
[-- Type: text/x-patch, Size: 7018 bytes --]

gdb/

        * linux-nat.c: Move common macros to ...
        Include linux-ptrace.h.
        * common/linux-ptrace.h: ... here.  New.

gdb/gdbserver/

        * linux-low.c: Move common macros to linux-ptrace.h.
        Include linux-ptrace.h.
	* Makefile.in (linux_ptrace_h): New.
	(linux-low.o): Depends on linux-ptrace.h.

diff --git a/gdb/common/linux-ptrace.h b/gdb/common/linux-ptrace.h
new file mode 100644
index 0000000..238a6f0
--- /dev/null
+++ b/gdb/common/linux-ptrace.h
@@ -0,0 +1,62 @@
+/* Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef COMMON_LINUX_PTRACE_H
+#define COMMON_LINUX_PTRACE_H
+
+#include <sys/ptrace.h>
+#include <sys/wait.h> /* __WAIT */
+
+#ifndef PTRACE_GETSIGINFO
+# define PTRACE_GETSIGINFO 0x4202
+# define PTRACE_SETSIGINFO 0x4203
+#endif /* PTRACE_GETSIGINF */
+
+/* If the system headers did not provide the constants, hard-code the normal
+   values.  */
+#ifndef PTRACE_EVENT_FORK
+
+#define PTRACE_SETOPTIONS	0x4200
+#define PTRACE_GETEVENTMSG	0x4201
+
+/* options set using PTRACE_SETOPTIONS */
+#define PTRACE_O_TRACESYSGOOD	0x00000001
+#define PTRACE_O_TRACEFORK	0x00000002
+#define PTRACE_O_TRACEVFORK	0x00000004
+#define PTRACE_O_TRACECLONE	0x00000008
+#define PTRACE_O_TRACEEXEC	0x00000010
+#define PTRACE_O_TRACEVFORKDONE	0x00000020
+#define PTRACE_O_TRACEEXIT	0x00000040
+
+/* Wait extended result codes for the above trace options.  */
+#define PTRACE_EVENT_FORK	1
+#define PTRACE_EVENT_VFORK	2
+#define PTRACE_EVENT_CLONE	3
+#define PTRACE_EVENT_EXEC	4
+#define PTRACE_EVENT_VFORK_DONE	5
+#define PTRACE_EVENT_EXIT	6
+
+#endif /* PTRACE_EVENT_FORK */
+
+/* We can't always assume that this flag is available, but all systems
+   with the ptrace event handlers also have __WALL, so it's safe to use
+   in some contexts.  */
+#ifndef __WALL
+#define __WALL          0x40000000 /* Wait for any child.  */
+#endif
+
+#endif /* COMMON_LINUX_PTRACE_H */
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 07e020f..6f4d31f 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -369,6 +369,8 @@ server_h = $(srcdir)/server.h $(regcache_h) config.h $(srcdir)/target.h \
 
 linux_low_h = $(srcdir)/linux-low.h
 
+linux_ptrace_h = $(srcdir)/../common/linux-ptrace.h
+
 lynx_low_h = $(srcdir)/lynx-low.h $(srcdir)/server.h
 
 nto_low_h = $(srcdir)/nto-low.h
@@ -437,7 +439,7 @@ i386-low.o: i386-low.c $(i386_low_h) $(server_h) $(target_h)
 
 i387-fp.o: i387-fp.c $(server_h)
 
-linux-low.o: linux-low.c $(linux_low_h) $(server_h)
+linux-low.o: linux-low.c $(linux_low_h) $(linux_ptrace_h) $(server_h)
 	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< @USE_THREAD_DB@
 
 linux-arm-low.o: linux-arm-low.c $(linux_low_h) $(server_h) \
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 93f3570..4470e3b 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -24,6 +24,7 @@
 #include <stdio.h>
 #include <sys/param.h>
 #include <sys/ptrace.h>
+#include "linux-ptrace.h"
 #include <signal.h>
 #include <sys/ioctl.h>
 #include <fcntl.h>
@@ -52,48 +53,10 @@
 #define SPUFS_MAGIC 0x23c9b64e
 #endif
 
-#ifndef PTRACE_GETSIGINFO
-# define PTRACE_GETSIGINFO 0x4202
-# define PTRACE_SETSIGINFO 0x4203
-#endif
-
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
 #endif
 
-/* If the system headers did not provide the constants, hard-code the normal
-   values.  */
-#ifndef PTRACE_EVENT_FORK
-
-#define PTRACE_SETOPTIONS	0x4200
-#define PTRACE_GETEVENTMSG	0x4201
-
-/* options set using PTRACE_SETOPTIONS */
-#define PTRACE_O_TRACESYSGOOD	0x00000001
-#define PTRACE_O_TRACEFORK	0x00000002
-#define PTRACE_O_TRACEVFORK	0x00000004
-#define PTRACE_O_TRACECLONE	0x00000008
-#define PTRACE_O_TRACEEXEC	0x00000010
-#define PTRACE_O_TRACEVFORKDONE	0x00000020
-#define PTRACE_O_TRACEEXIT	0x00000040
-
-/* Wait extended result codes for the above trace options.  */
-#define PTRACE_EVENT_FORK	1
-#define PTRACE_EVENT_VFORK	2
-#define PTRACE_EVENT_CLONE	3
-#define PTRACE_EVENT_EXEC	4
-#define PTRACE_EVENT_VFORK_DONE	5
-#define PTRACE_EVENT_EXIT	6
-
-#endif /* PTRACE_EVENT_FORK */
-
-/* We can't always assume that this flag is available, but all systems
-   with the ptrace event handlers also have __WALL, so it's safe to use
-   in some contexts.  */
-#ifndef __WALL
-#define __WALL          0x40000000 /* Wait for any child.  */
-#endif
-
 #ifndef W_STOPCODE
 #define W_STOPCODE(sig) ((sig) << 8 | 0x7f)
 #endif
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index cd88df8..b2860fb 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -30,6 +30,7 @@
 #endif
 #include <sys/ptrace.h>
 #include "linux-nat.h"
+#include "linux-ptrace.h"
 #include "linux-fork.h"
 #include "gdbthread.h"
 #include "gdbcmd.h"
@@ -160,49 +161,11 @@ blocked.  */
 #define O_LARGEFILE 0
 #endif
 
-/* If the system headers did not provide the constants, hard-code the normal
-   values.  */
-#ifndef PTRACE_EVENT_FORK
-
-#define PTRACE_SETOPTIONS	0x4200
-#define PTRACE_GETEVENTMSG	0x4201
-
-/* Options set using PTRACE_SETOPTIONS.  */
-#define PTRACE_O_TRACESYSGOOD	0x00000001
-#define PTRACE_O_TRACEFORK	0x00000002
-#define PTRACE_O_TRACEVFORK	0x00000004
-#define PTRACE_O_TRACECLONE	0x00000008
-#define PTRACE_O_TRACEEXEC	0x00000010
-#define PTRACE_O_TRACEVFORKDONE	0x00000020
-#define PTRACE_O_TRACEEXIT	0x00000040
-
-/* Wait extended result codes for the above trace options.  */
-#define PTRACE_EVENT_FORK	1
-#define PTRACE_EVENT_VFORK	2
-#define PTRACE_EVENT_CLONE	3
-#define PTRACE_EVENT_EXEC	4
-#define PTRACE_EVENT_VFORK_DONE	5
-#define PTRACE_EVENT_EXIT	6
-
-#endif /* PTRACE_EVENT_FORK */
-
 /* Unlike other extended result codes, WSTOPSIG (status) on
    PTRACE_O_TRACESYSGOOD syscall events doesn't return SIGTRAP, but
    instead SIGTRAP with bit 7 set.  */
 #define SYSCALL_SIGTRAP (SIGTRAP | 0x80)
 
-/* We can't always assume that this flag is available, but all systems
-   with the ptrace event handlers also have __WALL, so it's safe to use
-   here.  */
-#ifndef __WALL
-#define __WALL          0x40000000 /* Wait for any child.  */
-#endif
-
-#ifndef PTRACE_GETSIGINFO
-# define PTRACE_GETSIGINFO    0x4202
-# define PTRACE_SETSIGINFO    0x4203
-#endif
-
 /* The single-threaded native GNU/Linux target_ops.  We save a pointer for
    the use of the multi-threaded target.  */
 static struct target_ops *linux_ops;

Ping: Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

On 02/20/2011 11:05 PM, Yao Qi wrote:
> All the comments in previous mails are address in this new patch,
> 1.  rename 'linux-common.h' to 'linux-ptrace.h', which includes only
> ptrace-related macros.
> 2.  include <sys/wait.h> in linux-ptrace.h for macro __WALL,
> 3.  add dependency tracking for gdbserver,

> gdb/
> 
>         * linux-nat.c: Move common macros to ...
>         Include linux-ptrace.h.
>         * common/linux-ptrace.h: ... here.  New.
> 
> gdb/gdbserver/
> 
>         * linux-low.c: Move common macros to linux-ptrace.h.
>         Include linux-ptrace.h.
> 	* Makefile.in (linux_ptrace_h): New.
> 	(linux-low.o): Depends on linux-ptrace.h.
> 

Ping.

-- 
Yao (齐尧)

question about the common/ subdir (was "Re: Ping: Merge duplicated macros in linux-nat.c and linux-low.c")

[Joel Brobecker]

> > gdb/
> > 
> >         * linux-nat.c: Move common macros to ...
> >         Include linux-ptrace.h.
> >         * common/linux-ptrace.h: ... here.  New.
> > 
> > gdb/gdbserver/
> > 
> >         * linux-low.c: Move common macros to linux-ptrace.h.
> >         Include linux-ptrace.h.
> > 	* Makefile.in (linux_ptrace_h): New.
> > 	(linux-low.o): Depends on linux-ptrace.h.

Speaking of which, what was the latest decision regarding the
way we would handle the sources in common/.  I thought that we
were going to delete the configury and Makefile, and treat this
the same way we treat the gnulib/ directory.  Was that ever
decided? I personally would like to give this idea a try and
see where it goes, but I'm not enough of an expert to really
predict whether it's going to be better or not.  I can look
at producing patches, though.

-- 
Joel

Re: question about the common/ subdir (was "Re: Ping: Merge duplicated macros in linux-nat.c and linux-low.c")

[Yao Qi]

On 02/28/2011 10:06 PM, Joel Brobecker wrote:

> 
> Speaking of which, what was the latest decision regarding the
> way we would handle the sources in common/.  I thought that we
> were going to delete the configury and Makefile, and treat this
> the same way we treat the gnulib/ directory.  Was that ever
> decided? I personally would like to give this idea a try and

Personally, I don't want to see that happen, :)
I've given my cents here.
http://sourceware.org/ml/gdb-patches/2011-02/msg00669.html

> see where it goes, but I'm not enough of an expert to really
> predict whether it's going to be better or not.  I can look
> at producing patches, though.
> 

-- 
Yao (齐尧)

Re: question about the common/ subdir (was "Re: Ping: Merge duplicated macros in linux-nat.c and linux-low.c")

[Pedro Alves]

On Monday 28 February 2011 14:06:39, Joel Brobecker wrote:
> > > gdb/
> > > 
> > >         * linux-nat.c: Move common macros to ...
> > >         Include linux-ptrace.h.
> > >         * common/linux-ptrace.h: ... here.  New.
> > > 
> > > gdb/gdbserver/
> > > 
> > >         * linux-low.c: Move common macros to linux-ptrace.h.
> > >         Include linux-ptrace.h.
> > > 	* Makefile.in (linux_ptrace_h): New.
> > > 	(linux-low.o): Depends on linux-ptrace.h.
> 
> Speaking of which, what was the latest decision regarding the
> way we would handle the sources in common/.  I thought that we
> were going to delete the configury and Makefile, and treat this
> the same way we treat the gnulib/ directory.  Was that ever
> decided? I personally would like to give this idea a try and
> see where it goes, but I'm not enough of an expert to really
> predict whether it's going to be better or not.  I can look
> at producing patches, though.

IMO, we should go ahead with
that, see <http://sourceware.org/ml/gdb-patches/2011-02/msg00657.html>.

In sum, I'm convinced the trouble of listing an object
in two Makefiles is negligiceably compared to the pain
we've been getting ourselves into as long as:

 - the core set of headers between gdb and gdbserver aren't
   harmonized/shared, and,

 - we still need to maintain separate AC_CHECK_HEADERS & co
   in gdb's and gdbserver's configury&makefilery.
   This one is a major point against the current status
   quo, IMO.

I've pointed out Yet Another Way to handle this in the
url above (but as I said there, I'm not sure we want
to be playing with this stuff at this time).  And
it may be well be a totally stupid idea.  :-)

-- 
Pedro Alves

Re: question about the common/ subdir (was "Re: Ping: Merge duplicated macros in linux-nat.c and linux-low.c")

[Pedro Alves]

On Monday 28 February 2011 14:35:04, Yao Qi wrote:
> Personally, I don't want to see that happen, :)
> I've given my cents here.
> http://sourceware.org/ml/gdb-patches/2011-02/msg00669.html

Please post that patch on its own thread, this thread
is getting confusing, and I think that one can be
handled independently of what happens
to configure&make.

-- 
Pedro Alves

Re: Ping: Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

On 02/28/2011 08:36 PM, Yao Qi wrote:
> On 02/20/2011 11:05 PM, Yao Qi wrote:
>> All the comments in previous mails are address in this new patch,
>> 1.  rename 'linux-common.h' to 'linux-ptrace.h', which includes only
>> ptrace-related macros.
>> 2.  include <sys/wait.h> in linux-ptrace.h for macro __WALL,
>> 3.  add dependency tracking for gdbserver,
> 
>> gdb/
>>
>>         * linux-nat.c: Move common macros to ...
>>         Include linux-ptrace.h.
>>         * common/linux-ptrace.h: ... here.  New.
>>
>> gdb/gdbserver/
>>
>>         * linux-low.c: Move common macros to linux-ptrace.h.
>>         Include linux-ptrace.h.
>> 	* Makefile.in (linux_ptrace_h): New.
>> 	(linux-low.o): Depends on linux-ptrace.h.
>>
> 
> Ping.
> 

Ping?

Since configure/make issues in common/ has been resolved, can I apply
this patch to mainline?  Or apply it after 7.3 branch is created?

-- 
Yao (齐尧)

Re: Ping: Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

On 03/11/2011 12:11 PM, Yao Qi wrote:
> Ping?
> 
> Since configure/make issues in common/ has been resolved, can I apply
> this patch to mainline?  Or apply it after 7.3 branch is created?

Ping.

-- 
Yao (齐尧)

Re: Ping: Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

On 03/11/2011 12:11 PM, Yao Qi wrote:
> On 02/28/2011 08:36 PM, Yao Qi wrote:
>> On 02/20/2011 11:05 PM, Yao Qi wrote:
>>> All the comments in previous mails are address in this new patch,
>>> 1.  rename 'linux-common.h' to 'linux-ptrace.h', which includes only
>>> ptrace-related macros.
>>> 2.  include <sys/wait.h> in linux-ptrace.h for macro __WALL,
>>> 3.  add dependency tracking for gdbserver,
>>
>>> gdb/
>>>
>>>         * linux-nat.c: Move common macros to ...
>>>         Include linux-ptrace.h.
>>>         * common/linux-ptrace.h: ... here.  New.
>>>
>>> gdb/gdbserver/
>>>
>>>         * linux-low.c: Move common macros to linux-ptrace.h.
>>>         Include linux-ptrace.h.
>>> 	* Makefile.in (linux_ptrace_h): New.
>>> 	(linux-low.o): Depends on linux-ptrace.h.
>>>
>>
>> Ping.
>>
> 
> Ping?
> 
> Since configure/make issues in common/ has been resolved, can I apply
> this patch to mainline?  Or apply it after 7.3 branch is created?
> 

Ping.  7.3 branch was created.  OK for trunk?

-- 
Yao (齐尧)

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Tom Tromey]

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

Yao> All the comments in previous mails are address in this new patch,
Yao> 1.  rename 'linux-common.h' to 'linux-ptrace.h', which includes only
Yao> ptrace-related macros.
Yao> 2.  include <sys/wait.h> in linux-ptrace.h for macro __WALL,
Yao> 3.  add dependency tracking for gdbserver,

Yao> Ping.

Yao> Ping?

Yao> Ping.  7.3 branch was created.  OK for trunk?

Thanks for persevering.

I re-read the previous threads on this.  My understanding is that Mark
is uncertain about the general direction of this, but that neither he
nor anybody else objects to this specific patch.

I read the patch and I think merging common macro definitions makes
sense.

So, this is ok.

Tom

Re: [common] Merge duplicated macros in linux-nat.c and linux-low.c

[Yao Qi]

On 04/26/2011 01:03 AM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
> 
> 
> Thanks for persevering.
> 
> I re-read the previous threads on this.  My understanding is that Mark
> is uncertain about the general direction of this, but that neither he
> nor anybody else objects to this specific patch.
> 
> I read the patch and I think merging common macro definitions makes
> sense.
> 
> So, this is ok.

Thanks for the approval.  Run regression test for both native gdb and
native gdbserver on x86_64-linux.  No regression.  Applied to trunk.

http://sourceware.org/ml/gdb-cvs/2011-04/msg00161.html

-- 
Yao (齐尧)