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

[Gary Benson <gbenson@redhat.com>]

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

-- 
http://gbenson.net/