On Fri, Feb 28, 2014 at 11:17:41AM -0600, Eric Sandeen wrote: > On 2/28/14, 10:11 AM, Lukas Czerner wrote: > > Move the inclusion of falloc.h with all it's possible defines for the > > fallocate mode into global.h header file so we do not have to include > > and define it manually in every tool using fallocate. > > > > Signed-off-by: Lukas Czerner <lczerner@xxxxxxxxxx> > > I like the direction, but I think this changes behavior a little bit. > > #ifdef FALLOCATE came from an autoconf macro: > > AC_DEFUN([AC_PACKAGE_WANT_FALLOCATE], > [ AC_MSG_CHECKING([for fallocate]) > AC_TRY_LINK([ > #define _GNU_SOURCE > #define _FILE_OFFSET_BITS 64 > #include <fcntl.h> > #include <linux/falloc.h> ], > [ fallocate(0, 0, 0, 0); ], > [ have_fallocate=true; AC_MSG_RESULT(yes) ], > [ have_fallocate=false; AC_MSG_RESULT(no) ]) > AC_SUBST(have_fallocate) > ]) > > (at least I think so?) Not quite. autoconf defines "have_fallocate" to match the variable name in the AC_SUBST() macro above. The makefiles do this: include/builddefs.in:HAVE_FALLOCATE = @have_fallocate@ include/builddefs.in:HAVE_FALLOCATE = @have_fallocate@ to define HAVE_FALLOCATE at the makefile level, and then they do this to pass it into the C source: ltp/Makefile:ifeq ($(HAVE_FALLOCATE), true) ltp/Makefile:LCFLAGS += -DFALLOCATE src/Makefile:ifeq ($(HAVE_FALLOCATE), true) src/Makefile:LCFLAGS += -DHAVE_FALLOCATE > and so #ifdef FALLOCATE meant that > an fallocate syscall actually exists. With your changes, > the test is now whether the fallocate *header* exists. It actually tests both, because if header doesn't exist, the compile of the test stub will fail in the macro will fail. So, no change there, really. > falloc.h is part of kernel-headers, not glibc. So it's > possible that there's a divergence between the two. Right, which is why we need to test both ;) > I think it's probably ok. Build-time checks should > determine whether we are able to _build_ and yours do that. > Each caller of fallocate (or each test using it) then probably > needs to ensure that the functionality it wants is actually > available at runtime and handle it if not. > > So I'll give this a > > Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx> > > but maybe the above rambling will ring alarm bells for > someone else... ;) I need to look at it all in more detail. I thought I'd just explain exactly what was happening with autoconf here... Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
