[PATCH] correct order of include files to include replace.h first
Jeremy Allison
jra at samba.org
Thu Jul 16 22:42:02 UTC 2015
On Sun, Jul 12, 2015 at 08:27:09PM +0300, Uri Simchoni wrote:
> 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
Reviewed-by: Jeremy Allison <jra at samba.org>
Can I get a second Team reviewer ?
> 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
> >
> 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