linux-ext4 - Re: [PATCH 4/8] xfstests: Move fallocate include into global.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140228223105.GE13647@dastard>
Date:	Sat, 1 Mar 2014 09:31:05 +1100
From:	Dave Chinner <david@...morbit.com>
To:	Eric Sandeen <sandeen@...deen.net>
Cc:	Lukas Czerner <lczerner@...hat.com>, linux-ext4@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, xfs@....sgi.com
Subject: Re: [PATCH 4/8] xfstests: Move fallocate include into global.h

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@...hat.com>
> 
> 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@...hat.com>
> 
> 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@...morbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html