diff mbox

[v6,04/10] Create empty common/linux-maps.[ch] and common/target-utils.[ch]

Message ID 20150607200454.8918.52868.stgit@host1.jankratochvil.net
State New
Headers show

Commit Message

Jan Kratochvil June 7, 2015, 8:04 p.m. UTC
Hi,

prepare new files for later move.

Approved by:
	https://sourceware.org/ml/gdb-patches/2014-05/msg00367.html


Jan


gdb/ChangeLog
2014-02-26  Aleksandar Ristovski  <aristovski@qnx.com
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Create empty common/linux-maps.[ch] and common/target-utils.[ch].
	* Makefile.in (HFILES_NO_SRCDIR); Add common/linux-maps.h,
	common/target-utils.h.
	(COMMON_OBS): Add target-utils.o.
	(linux-maps.o, target-utils.o): New.
	* common/linux-maps.c: New file.
	* common/linux-maps.h: New file.
	* common/target-utils.c: New file.
	* common/target-utils.h: New file.
	* config/i386/linux.mh (NATDEPFILES): Add linux-maps.o.
	* config/i386/linux64.mh (NATDEPFILES): Ditto.

gdb/gdbserver/ChangeLog
2014-02-26  Aleksandar Ristovski  <aristovski@qnx.com
	    Jan Kratochvil  <jan.kratochvil@redhat.com>

	Create empty common/linux-maps.[ch] and common/target-utils.[ch].
	* Makefile.in (OBS): Add target-utils.o.
	(linux-maps.o, target-utils.o): New.
	* configure.srv (srv_linux_obj): Add linux-maps.o.
---
 gdb/Makefile.in             |   14 +++++++++++---
 gdb/common/linux-maps.c     |   25 +++++++++++++++++++++++++
 gdb/common/linux-maps.h     |   22 ++++++++++++++++++++++
 gdb/common/target-utils.c   |   26 ++++++++++++++++++++++++++
 gdb/common/target-utils.h   |   23 +++++++++++++++++++++++
 gdb/config/i386/linux.mh    |    2 +-
 gdb/config/i386/linux64.mh  |    2 +-
 gdb/gdbserver/Makefile.in   |    8 +++++++-
 gdb/gdbserver/configure.srv |    2 +-
 9 files changed, 117 insertions(+), 7 deletions(-)
 create mode 100644 gdb/common/linux-maps.c
 create mode 100644 gdb/common/linux-maps.h
 create mode 100644 gdb/common/target-utils.c
 create mode 100644 gdb/common/target-utils.h

Comments

Gary Benson June 8, 2015, 8:37 a.m. UTC | #1
Jan Kratochvil wrote:
> prepare new files for later move.
> 
> Approved by:
> 	https://sourceware.org/ml/gdb-patches/2014-05/msg00367.html

Like Tom, I too find it weird having a commit that creates empty
files.  I'd prefer to see the new files created fully populated.

A bunch of stuff has changed in the way common code is laid out since
May 2014, so while Tom approved this back then, it's not suitable any
more.  Please update the series as follows:

> 	* common/linux-maps.c: New file.
> 	* common/linux-maps.h: New file.

Nothing os-specific should be in common.  These files should be
nat/linux-maps.[ch].

> 	* common/target-utils.c: New file.
> 	* common/target-utils.h: New file.

Nothing to do with the target should be in common.  The declarations
should probably be in target/target.h, and they should have "target_"
prefixes.  You could create target/target.c to put the definitions in.

> diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
...
> +
> +#ifdef GDBSERVER
> +#include "server.h"
> +#else
> +#include "defs.h"
> +#endif
> +
> +#include "linux-maps.h"

This should be:

  #include "common-defs.h"
  #include "linux-maps.h"

Please read this:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Include_Files

Other new files in this patch need similar treatment.

On a separate but related note, please do not add GDBSERVER
conditionals anywhere, I spent half a year removing almost
all of them and the remaining couple are on my hit list.

Thanks,
Gary
Jan Kratochvil June 11, 2015, 6:48 p.m. UTC | #2
On Mon, 08 Jun 2015 10:37:33 +0200, Gary Benson wrote:
> Jan Kratochvil wrote:
> > 	* common/target-utils.c: New file.
> > 	* common/target-utils.h: New file.
> 
> Nothing to do with the target should be in common.  The declarations
> should probably be in target/target.h, and they should have "target_"
> prefixes.  You could create target/target.c to put the definitions in.

gdb/target/target.c would create target.o which would conflict with
gdb/target.c.

There is already gdb/target/target.h "conflicting" with gdb/target.h but it
does not really conflict as gdb/target/ does not have default -I include dir.
Despite for example gdb/common/ has default -I.

I could propose various ideas what to move there but what is the suggested
solution?  Sure one could also for example just generate gdb/target-target.o
from gdb/target/target.c.


Jan
Aleksandar Ristovski June 11, 2015, 7:46 p.m. UTC | #3
On 2015-06-11, 2:48 PM, "Jan Kratochvil" <jan.kratochvil@redhat.com> wrote:

>On Mon, 08 Jun 2015 10:37:33 +0200, Gary Benson wrote:
>> Jan Kratochvil wrote:
>> > 	* common/target-utils.c: New file.
>> > 	* common/target-utils.h: New file.
>> 
>> Nothing to do with the target should be in common.  The declarations
>> should probably be in target/target.h, and they should have "target_"
>> prefixes.  You could create target/target.c to put the definitions in.
>
>gdb/target/target.c would create target.o which would conflict with
>gdb/target.c.
>
>There is already gdb/target/target.h "conflicting" with gdb/target.h but
>it
>does not really conflict as gdb/target/ does not have default -I include
>dir.
>Despite for example gdb/common/ has default -I.
>
>I could propose various ideas what to move there but what is the suggested
>solution?  Sure one could also for example just generate
>gdb/target-target.o
>from gdb/target/target.c.

As you say, there could be various ideas if needed, but the simplest seems
good enough to me: just move the files to target dir

common/target-utils.[hc] -> target/target_utils.[hc]

Provided Gary is happy with that solution.
Gary Benson June 12, 2015, 11:26 a.m. UTC | #4
Aleksandar Ristovski wrote:
> On 2015-06-11, 2:48 PM, "Jan Kratochvil" <jan.kratochvil@redhat.com> wrote:
> >On Mon, 08 Jun 2015 10:37:33 +0200, Gary Benson wrote:
> > > Jan Kratochvil wrote:
> > > > 	* common/target-utils.c: New file.
> > > > 	* common/target-utils.h: New file.
> > > 
> > > Nothing to do with the target should be in common.  The
> > > declarations should probably be in target/target.h, and they
> > > should have "target_" prefixes.  You could create
> > > target/target.c to put the definitions in.
> >
> > gdb/target/target.c would create target.o which would conflict
> > with gdb/target.c.
> > 
> > There is already gdb/target/target.h "conflicting" with
> > gdb/target.h but it does not really conflict as gdb/target/ does
> > not have default -I include dir.  Despite for example gdb/common/
> > has default -I.

-Igdb/common is going to be removed at some stage.  I have a script
to do it, I just didn't ever get around to it.  Maybe once 7.10 is
released...

> > I could propose various ideas what to move there but what is the
> > suggested solution?  Sure one could also for example just generate
> > gdb/target-target.o from gdb/target/target.c.
> 
> As you say, there could be various ideas if needed, but the simplest
> seems good enough to me: just move the files to target dir
> 
> common/target-utils.[hc] -> target/target_utils.[hc]
> 
> Provided Gary is happy with that solution.

Works for me :)

Thanks,
Gary
Jan Kratochvil June 14, 2015, 7:28 p.m. UTC | #5
On Mon, 08 Jun 2015 10:37:33 +0200, Gary Benson wrote:
> Nothing to do with the target should be in common.  The declarations
> should probably be in target/target.h, and they should have "target_"
> prefixes.  You could create target/target.c to put the definitions in.

Created target/target-utils.c as we discussed elsewhere.


> > diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
> ...
> > +
> > +#ifdef GDBSERVER
> > +#include "server.h"
> > +#else
> > +#include "defs.h"
> > +#endif
> > +
> > +#include "linux-maps.h"
> 
> This should be:
> 
>   #include "common-defs.h"
>   #include "linux-maps.h"

Done.


> On a separate but related note, please do not add GDBSERVER
> conditionals anywhere, I spent half a year removing almost
> all of them and the remaining couple are on my hit list.

Done.


Jan
Gary Benson June 23, 2015, 8:40 a.m. UTC | #6
Jan Kratochvil wrote:
> On Mon, 08 Jun 2015 10:37:33 +0200, Gary Benson wrote:
> > Nothing to do with the target should be in common.  The
> > declarations should probably be in target/target.h, and
> > they should have "target_" prefixes.  You could create
> > target/target.c to put the definitions in.
> 
> Created target/target-utils.c as we discussed elsewhere.
> 
> > > diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
> > ...
> > > +
> > > +#ifdef GDBSERVER
> > > +#include "server.h"
> > > +#else
> > > +#include "defs.h"
> > > +#endif
> > > +
> > > +#include "linux-maps.h"
> > 
> > This should be:
> > 
> >   #include "common-defs.h"
> >   #include "linux-maps.h"
> 
> Done.
> 
> > On a separate but related note, please do not add GDBSERVER
> > conditionals anywhere, I spent half a year removing almost
> > all of them and the remaining couple are on my hit list.
> 
> Done.

Thanks Jan.

Cheers,
Gary
diff mbox

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 95104ef..3b1d1d8 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -900,7 +900,7 @@  LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
 # right, it is probably easiest just to list .h files here directly.
 
 HFILES_NO_SRCDIR = \
-common/gdb_signals.h nat/gdb_thread_db.h common/gdb_vecs.h \
+common/gdb_signals.h nat/gdb_thread_db.h common/gdb_vecs.h common/linux-maps.h \
 common/x86-xstate.h nat/linux-ptrace.h nat/mips-linux-watch.h \
 proc-utils.h aarch64-tdep.h arm-tdep.h ax-gdb.h ppcfbsd-tdep.h \
 ppcnbsd-tdep.h cli-out.h gdb_expat.h breakpoint.h infcall.h obsd-tdep.h \
@@ -979,7 +979,7 @@  i386-linux-nat.h common/common-defs.h common/errors.h common/common-types.h \
 common/common-debug.h common/cleanups.h common/gdb_setjmp.h \
 common/common-exceptions.h target/target.h common/symbol.h \
 common/common-regcache.h fbsd-tdep.h nat/linux-personality.h \
-common/fileio.h nat/x86-linux.h nat/x86-linux-dregs.h
+common/fileio.h nat/x86-linux.h nat/x86-linux-dregs.h common/target-utils.h
 
 # Header files that already have srcdir in them, or which are in objdir.
 
@@ -1079,7 +1079,7 @@  COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o \
 	format.o registry.o btrace.o record-btrace.o waitstatus.o \
 	print-utils.o rsp-low.o errors.o common-debug.o debug.o \
-	common-exceptions.o btrace-common.o fileio.o \
+	common-exceptions.o btrace-common.o fileio.o target-utils.o \
 	$(SUBDIR_GCC_COMPILE_OBS)
 
 TSOBS = inflow.o
@@ -2217,6 +2217,10 @@  common-agent.o: $(srcdir)/common/agent.c
 	$(COMPILE) $(srcdir)/common/agent.c
 	$(POSTCOMPILE)
 
+linux-maps.o: ${srcdir}/common/linux-maps.c
+	$(COMPILE) $(srcdir)/common/linux-maps.c
+	$(POSTCOMPILE)
+
 vec.o: ${srcdir}/common/vec.c
 	$(COMPILE) $(srcdir)/common/vec.c
 	$(POSTCOMPILE)
@@ -2233,6 +2237,10 @@  errors.o: ${srcdir}/common/errors.c
 	$(COMPILE) $(srcdir)/common/errors.c
 	$(POSTCOMPILE)
 
+target-utils.o: ${srcdir}/common/target-utils.c
+	$(COMPILE) $(srcdir)/common/target-utils.c
+	$(POSTCOMPILE)
+
 common-debug.o: ${srcdir}/common/common-debug.c
 	$(COMPILE) $(srcdir)/common/common-debug.c
 	$(POSTCOMPILE)
diff --git a/gdb/common/linux-maps.c b/gdb/common/linux-maps.c
new file mode 100644
index 0000000..bb3eac9
--- /dev/null
+++ b/gdb/common/linux-maps.c
@@ -0,0 +1,25 @@ 
+/* Linux-specific memory maps manipulation routines.
+   Copyright (C) 2014 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/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include "linux-maps.h"
diff --git a/gdb/common/linux-maps.h b/gdb/common/linux-maps.h
new file mode 100644
index 0000000..ebf6f37
--- /dev/null
+++ b/gdb/common/linux-maps.h
@@ -0,0 +1,22 @@ 
+/* Linux-specific memory maps manipulation routines.
+   Copyright (C) 2014 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_MAPS_H
+#define COMMON_LINUX_MAPS_H
+
+#endif /* COMMON_LINUX_MAPS_H */
diff --git a/gdb/common/target-utils.c b/gdb/common/target-utils.c
new file mode 100644
index 0000000..308996d
--- /dev/null
+++ b/gdb/common/target-utils.c
@@ -0,0 +1,26 @@ 
+/* Utility target functions for GDB, and GDBserver.
+
+   Copyright (C) 2014 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/>.  */
+
+#ifdef GDBSERVER
+#include "server.h"
+#else
+#include "defs.h"
+#endif
+
+#include "target-utils.h"
diff --git a/gdb/common/target-utils.h b/gdb/common/target-utils.h
new file mode 100644
index 0000000..db33ec8
--- /dev/null
+++ b/gdb/common/target-utils.h
@@ -0,0 +1,23 @@ 
+/* Utility target functions for GDB, and GDBserver.
+
+   Copyright (C) 2014 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_TARGET_UTILS_H
+#define COMMON_TARGET_UTILS_H
+
+#endif /* COMMON_TARGET_UTILS_H */
diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
index 9be2c5f..f650005 100644
--- a/gdb/config/i386/linux.mh
+++ b/gdb/config/i386/linux.mh
@@ -3,7 +3,7 @@ 
 NAT_FILE= config/nm-linux.h
 NATDEPFILES= inf-ptrace.o fork-child.o \
 	x86-nat.o x86-dregs.o i386-linux-nat.o x86-linux-nat.o \
-	proc-service.o linux-thread-db.o \
+	proc-service.o linux-thread-db.o linux-maps.o \
 	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
 	linux-btrace.o linux-waitpid.o linux-personality.o x86-linux.o \
 	x86-linux-dregs.o
diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
index 224a2a9..b71bda0 100644
--- a/gdb/config/i386/linux64.mh
+++ b/gdb/config/i386/linux64.mh
@@ -2,7 +2,7 @@ 
 NATDEPFILES= inf-ptrace.o fork-child.o \
 	x86-nat.o x86-dregs.o amd64-nat.o amd64-linux-nat.o \
 	x86-linux-nat.o \
-	linux-nat.o linux-osdata.o \
+	linux-maps.o linux-nat.o linux-osdata.o \
 	proc-service.o linux-thread-db.o linux-fork.o \
 	linux-procfs.o linux-ptrace.o linux-btrace.o \
 	linux-waitpid.o linux-personality.o x86-linux.o \
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index a981ee8..b4465e6 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -194,7 +194,7 @@  OBS = agent.o ax.o inferiors.o regcache.o remote-utils.o server.o signals.o \
       mem-break.o hostio.o event-loop.o tracepoint.o xml-utils.o \
       common-utils.o ptid.o buffer.o format.o filestuff.o dll.o notif.o \
       tdesc.o print-utils.o rsp-low.o errors.o common-debug.o cleanups.o \
-      common-exceptions.o symbol.o btrace-common.o fileio.o \
+      common-exceptions.o symbol.o btrace-common.o fileio.o target-utils.o \
       $(XML_BUILTIN) $(DEPFILES) $(LIBOBJS)
 GDBREPLAY_OBS = gdbreplay.o version.o
 GDBSERVER_LIBS = @GDBSERVER_LIBS@
@@ -519,6 +519,9 @@  ax.o: ax.c
 signals.o: ../common/signals.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+linux-maps.o: ../common/linux-maps.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 print-utils.o: ../common/print-utils.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
@@ -531,6 +534,9 @@  common-utils.o: ../common/common-utils.c
 posix-strerror.o: ../common/posix-strerror.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
+target-utils.o: ../common/target-utils.c
+	$(COMPILE) $<
+	$(POSTCOMPILE)
 mingw-strerror.o: ../common/mingw-strerror.c
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 81dd235..3b3888d 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -42,7 +42,7 @@  srv_amd64_linux_xmlfiles="i386/amd64-linux.xml i386/amd64-avx-linux.xml i386/amd
 
 # Linux object files.  This is so we don't have to repeat
 # these files over and over again.
-srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-waitpid.o linux-personality.o"
+srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-maps.o linux-ptrace.o linux-waitpid.o linux-personality.o"
 
 # Input is taken from the "${target}" variable.