[PATCH] correct order of include files to include replace.h first

[Uri Simchoni]

Well it so happens that we can keep the convention in this module also
- see attached amended patch. What we cannot do is include
"system/filesys.h" - it is not compatible with <linux/fs.h> because of
<sys/mount.h>. Therefore if this module ever ends up needing
<sys/something>, the convention will have to be broken.

In general I think we can cut some slack to plugin modules which
interface with external systems and have to somehow shoehorn both
Samba's include files and the external system's include files. In this
case it's true that the "external system" is the kernel, but the
plugin is not using the POSIX common denominator that is abstracted by
"system/filesys.h" - it is using Linux-specific interfaces. That's why
I wasn't trying too hard to make it work, but eventually it turns out
that it can obey the convention.

Error message - also fixed in amended patch.

Thanks,
Uri.

On Fri, Jul 17, 2015 at 2:26 AM, Jeremy Allison <jra at samba.org> wrote:
> On Fri, Jul 17, 2015 at 12:48:27AM +0200, Stefan (metze) Metzmacher wrote:
>> Hi Jeremy,
>> >> v2 7/8 - fix vfs_btrfs.c
>> >> v2 8/8 - change replace.h to prevent such errors in the future
>> >
>> > Reviewed-by: Jeremy Allison <jra at samba.org>
>> >
>> > Can I get a second Team reviewer ?
>>
>> See my comments below...
> ...
>> >>  #include <libgen.h>
>> >> -#include "system/filesys.h"
>> >> -#include "includes.h"
>>
>> This looks still wrong "includes.h" should be the first one
>> and "system/filesys.h" should be kept as 2nd, some of the other
>> might not be needed anymore then.
> ....
>>
>> #error "config.h must be included before any glibc header file"
>>
>> should be something like this:
>>
>> #error "config.h via replace.h must be included before any glibc header
>> file"
>>
>> in order to indicate that this check comes from replace.h.
>>
>> Otherwise Reviewed-by: Stefan Metzmacher <metze at samba.org>
>
> OK, I'll push everything except the last 2 and we can
> work those out together with Uri.
>
> Cheers,
>
>         Jeremy.
-------------- next part --------------
From 5e8ba223f9b03d2e31a651ec650776b390d17e19 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Sun, 12 Jul 2015 11:19:49 +0300
Subject: [PATCH v3 1/2] vfs_btrfs: include config.h before glibc headers

Comply with the convention of including config.h before any
glibc header file, to make config.h definitions such as
_FILE_OFFSET_BITS affect glibc.

This commit does not fix a specific known bug. It changes the code to
comply with coding conventions.

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 source3/modules/vfs_btrfs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
index bd95637..57571bb 100644
--- a/source3/modules/vfs_btrfs.c
+++ b/source3/modules/vfs_btrfs.c
@@ -17,15 +17,13 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "includes.h"
 #include <linux/ioctl.h>
 #include <linux/fs.h>
-#include <sys/ioctl.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <dirent.h>
 #include <libgen.h>
-#include "system/filesys.h"
-#include "includes.h"
 #include "smbd/smbd.h"
 #include "librpc/gen_ndr/smbXsrv.h"
 #include "librpc/gen_ndr/ioctl.h"
-- 
1.9.1


From 2ff79ffd3b2db7696f6669a38d790f5cffdc6094 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Sun, 12 Jul 2015 10:00:27 +0300
Subject: [PATCH v3 2/2] replace.h: verify that config.h is included first

Add a check before #including config.h that no glibc header has
already been included. The importance for this is that some definitions
in config.h affect glibc (e.g. _FILE_OFFSET_BITS) and if any glibc header
is already included then it's already too late.

Exempted from this check are files which also include Python devel
headers, because otherwise we have lots of "redefined" warnings, and
Python has its own config.h with definitions such as _FILE_OFFSET_BITS.

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 lib/replace/replace.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/replace/replace.h b/lib/replace/replace.h
index 3ff4e36..56094e4 100644
--- a/lib/replace/replace.h
+++ b/lib/replace/replace.h
@@ -29,6 +29,10 @@
 #define _LIBREPLACE_REPLACE_H
 
 #ifndef NO_CONFIG_H
+#if defined(_FEATURES_H) && !defined(Py_PYCONFIG_H)
+#error "config.h via replace.h must be included before any glibc header file"
+#endif
+
 #include "config.h"
 #endif
 
-- 
1.9.1