[PATCH] correct order of include files to include replace.h first
Uri Simchoni
urisimchoni at gmail.com
Sun Jul 12 17:27:09 UTC 2015
Hi.
Attached pls find fixed patch set. Changes are:
- Split the second patch into several patches, one patch per subdir
- Remove some included sys/xxx files
- Also fix vfs_btrfs.c include order. The main problem was that when
config.h is included correctly, then replace/system/filesys.h includes
<sys/mount.h>, and that clashes with <linux/fs.h>. Because vfs_btrfs.c
is linux-specific, I opted to removing system/filesys.h.
- To prevent this error from recurring, instead of changing the
generated config.h by waf - change replace.h
v2 1/8 - same as v1 1/3
v2 2/8 to 6/8 - a split of v1 2/3, also removing some system include files
v2 7/8 - fix vfs_btrfs.c
v2 8/8 - change replace.h to prevent such errors in the future
Thanks!
Uri
On Thu, Jul 9, 2015 at 12:55 AM, Stefan (metze) Metzmacher
<metze at samba.org> wrote:
> Hi Uri,
>
>> This is a re-submission of a patch series, because the previous
>> submission was part of a thread and may have gone unnoticed.
>> The patches change order of include files to make sure replace.h is
>> included first. I am told this is the convention.
>>
>> 1/3 - This is an actual bug fix, causing fcntl locking not to work in
>> 32-bit x86.
>
> Reviewed-by: me
>
>> 2/3 - Fix order of includes in other files, to comply with the
>> convention, but with no actual known bug that it fixes
>
> Can you please split this patch? One commit per sub directory please.
> This makes potential backporting easier. In some places you could
> also try to remove the direct include of system headers, we should
> try to use "system/*.h" instead of possible.
>
>> 3/3 - a change to the build system to catch all those files in 2/3 -
>> This is a change to waf so I don't really expect it to be pushed
>> as-is, it's just an idea of what can be done to automate this check.
>
> Why don't you fix source3/modules/vfs_btrfs.c
> similar to the 2nd patch.
>
> Instead of modifying third_party/waf/wafadmin/Tools/config_c.py,
> I guess we can add somthing like this to lib/replace/replace.h.
>
> metze
>
-------------- next part --------------
From 1ea7554ed87b622801e7d0602c659ae1b4c74921 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Thu, 2 Jul 2015 20:09:02 +0300
Subject: [PATCH v2 1/8] util.c: fix order of inclusion to correctly consume
config.h
replace.h has to be the first file included in order to correctly act
upon the definitions in config.h.
Specifically, this change fixes 32-bit i686 builds, which depend upon
_FILE_OFFSET_BITS=64 to be set before any standard library file is
included.
Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
lib/util/util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/util/util.c b/lib/util/util.c
index 4f0e67f..393d83d 100644
--- a/lib/util/util.c
+++ b/lib/util/util.c
@@ -22,8 +22,8 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <talloc.h>
#include "replace.h"
+#include <talloc.h>
#include "system/network.h"
#include "system/filesys.h"
#include "system/locale.h"
--
1.9.1
From 1461dd5caf08c40d89ea65d6e401ef1b53768345 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Sun, 12 Jul 2015 09:29:13 +0300
Subject: [PATCH v2 2/8] tdbrestore: include config.h before any glibc headers
config.h may have some flags which affect glibc behavior, e.g.
_FILE_OFFSET_BITS=64. To make sure these flags have the desired
effect, config.h must be included before any glibc header files.
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>
---
lib/tdb/tools/tdbrestore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/tdb/tools/tdbrestore.c b/lib/tdb/tools/tdbrestore.c
index f65b36f..81c986c 100644
--- a/lib/tdb/tools/tdbrestore.c
+++ b/lib/tdb/tools/tdbrestore.c
@@ -17,8 +17,8 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <assert.h>
#include "replace.h"
+#include <assert.h>
#include "system/locale.h"
#include "system/time.h"
#include "system/filesys.h"
--
1.9.1
From 04af42173b75f3592e043de0406171a85b502756 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Sun, 12 Jul 2015 09:30:36 +0300
Subject: [PATCH v2 3/8] lib/util: include config.h before any glibc headers
config.h may have some flags which affect glibc behavior, e.g.
_FILE_OFFSET_BITS=64. To make sure these flags have the desired
effect, config.h must be included before any glibc header files.
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>
---
lib/util/debug.c | 2 +-
lib/util/idtree.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/util/debug.c b/lib/util/debug.c
index 726c682..f2a445b 100644
--- a/lib/util/debug.c
+++ b/lib/util/debug.c
@@ -19,8 +19,8 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <talloc.h>
#include "replace.h"
+#include <talloc.h>
#include "system/filesys.h"
#include "system/syslog.h"
#include "system/locale.h"
diff --git a/lib/util/idtree.c b/lib/util/idtree.c
index 0056c09..2104c74 100644
--- a/lib/util/idtree.c
+++ b/lib/util/idtree.c
@@ -32,8 +32,8 @@
* @file
*/
-#include <talloc.h>
#include "replace.h"
+#include <talloc.h>
#include "debug.h"
#include "idtree.h"
--
1.9.1
From 8f27e3b042f510a37baab4250103028d6fd9f0bc Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Sun, 12 Jul 2015 09:31:52 +0300
Subject: [PATCH v2 4/8] source3/lib: include config.h before any glibc headers
config.h may have some flags which affect glibc behavior, e.g.
_FILE_OFFSET_BITS=64. To make sure these flags have the desired
effect, config.h must be included before any glibc header files.
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/lib/messages_dgm_ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/lib/messages_dgm_ref.c b/source3/lib/messages_dgm_ref.c
index b4511e0..0a6cbf7 100644
--- a/source3/lib/messages_dgm_ref.c
+++ b/source3/lib/messages_dgm_ref.c
@@ -17,8 +17,8 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <talloc.h>
#include "replace.h"
+#include <talloc.h>
#include "messages_dgm.h"
#include "messages_dgm_ref.h"
#include "lib/util/debug.h"
--
1.9.1
From a7ae4631a64b4c6786890a8671c0e6c337fb6bda Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Sun, 12 Jul 2015 09:36:46 +0300
Subject: [PATCH v2 5/8] fssd: include config.h before any glibc headers
config.h may have some flags which affect glibc behavior, e.g.
_FILE_OFFSET_BITS=64. To make sure these flags have the desired
effect, config.h must be included before any glibc header files.
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/rpc_server/fss/srv_fss_state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/rpc_server/fss/srv_fss_state.c b/source3/rpc_server/fss/srv_fss_state.c
index 97fd0cb..97604f3 100644
--- a/source3/rpc_server/fss/srv_fss_state.c
+++ b/source3/rpc_server/fss/srv_fss_state.c
@@ -17,8 +17,8 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <fcntl.h>
#include "source3/include/includes.h"
+#include <fcntl.h>
#include "source3/include/util_tdb.h"
#include "lib/dbwrap/dbwrap.h"
#include "lib/dbwrap/dbwrap_open.h"
--
1.9.1
From 68ddb73dd032473e551f10f0dd050ee29b2e50bf Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Sun, 12 Jul 2015 09:38:01 +0300
Subject: [PATCH v2 6/8] torture: include config.h before any glibc headers
config.h may have some flags which affect glibc behavior, e.g.
_FILE_OFFSET_BITS=64. To make sure these flags have the desired
effect, config.h must be included before any glibc header files.
Also remove inclusion of some system files, relying on
replace/system/*.h instead.
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>
---
source4/torture/local/fsrvp_state.c | 4 +---
source4/torture/local/verif_trailer.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/source4/torture/local/fsrvp_state.c b/source4/torture/local/fsrvp_state.c
index f9c0f0f..3a08543 100644
--- a/source4/torture/local/fsrvp_state.c
+++ b/source4/torture/local/fsrvp_state.c
@@ -17,11 +17,9 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <sys/types.h>
-#include <sys/stat.h>
+#include "includes.h"
#include <unistd.h>
-#include "includes.h"
#include "librpc/gen_ndr/security.h"
#include "lib/param/param.h"
#include "lib/util/dlinklist.h"
diff --git a/source4/torture/local/verif_trailer.c b/source4/torture/local/verif_trailer.c
index eb95b21..efb2ac7 100644
--- a/source4/torture/local/verif_trailer.c
+++ b/source4/torture/local/verif_trailer.c
@@ -19,11 +19,9 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
-#include <sys/types.h>
-#include <sys/stat.h>
+#include "includes.h"
#include <unistd.h>
-#include "includes.h"
#include "librpc/gen_ndr/security.h"
#include "lib/param/param.h"
#include "lib/util/dlinklist.h"
--
1.9.1
From 2fe336a614e8379bb12b378b47f2330d44f4131b 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 v2 7/8] 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 | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
index bd95637..3799757 100644
--- a/source3/modules/vfs_btrfs.c
+++ b/source3/modules/vfs_btrfs.c
@@ -19,13 +19,12 @@
#include <linux/ioctl.h>
#include <linux/fs.h>
+#include "includes.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 ff39fa49497a8e66c0467e8f7be7e47d22979b88 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 v2 8/8] 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..6f407e9 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 must be included before any glibc header file"
+#endif
+
#include "config.h"
#endif
--
1.9.1
More information about the samba-technical
mailing list