Re: [LTP] [PATCH] configure: add configure checks to compile kernel modules

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1.1: Type: Text/Plain, Size: 2676 bytes --]

On Thursday 06 June 2013 10:21:47 Alexey Kodanev wrote:
> --- /dev/null
> +++ b/m4/ltp-kernel_devel.m4
> @@ -0,0 +1,72 @@
> +dnl Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
> +dnl
> +dnl This program is free software; you can redistribute it and/or
> +dnl modify it under the terms of the GNU General Public License as
> +dnl published by the Free Software Foundation.
> +dnl
> +dnl This program is distributed in the hope that it would be useful,
> +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
> +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +dnl GNU General Public License for more details.
> +dnl
> +dnl You should have received a copy of the GNU General Public License
> +dnl along with this program; if not, write the Free Software Foundation,
> +dnl Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA

you've apparently deleted the phrase "either version 2 of the License, or (at 
your option) any later version.".  i have no idea why you would do such a 
thing, but it's obviously not desirable.  apparently this isn't the first time 
as i see you've done it in at least 0e335dbed65afb79eab5b30e8d49d8066f165316.

please send a patch to fix your previous commits, and fix this one as well.  or 
i'm afraid we'll have to revert.

> +AC_DEFUN([LTP_CHECK_KERNEL_DEVEL],
> +[

cuddle the [ up and use dnl.  see _LTP_CHECK_LINUX_PTRACE for what i mean.

> +AC_ARG_WITH(
> +	    [kbuild-release],

"kbuild "is meaningless outside of the linux kernel tree.  it's also not a 
specific release.  call it "linux-version" instead.  or maybe "kmod-version".

> +	    [AC_HELP_STRING([--with-kbuild-release=RELEASE],
> +			    [specify the kernel release to build modules for])],

you're mixing tabs & spaces in a way that only works with 8 spaces.  either 
use pure tabs to indent (one level at a time), or use tabs to line up to the 
common part, then use spaces after that.

> +	    [KBUILD_RELEASE="${withval}"],
> +	    [KBUILD_RELEASE=`uname -r`])

defaulting like this won't work.  you need to check $cross_compiling is set to 
yes in order to default to `uname -r`.

seems like it'd be simpler to just do:
	AC_ARG_WITH([linux-version], [AC_HELP_STRING(...)],,
		AS_IF([test "$cross_compiling" != no],
			[with_linux_version=`uname -r`]))
	LINUX_VERSION=$with_linux_version
	AC_SUBST(LINUX_VERSION)

> +AC_MSG_RESULT([checking for kernel-devel... $WITH_MODULES])

err, that's not how it works.  you first call AC_MSG_CHECKING([for kernel-
devel]) and then you do AC_MSG_RESULT([$WITH_MODULES]) after the actual 
checks.
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 184 bytes --]

------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev

[-- Attachment #3: Type: text/plain, Size: 155 bytes --]

_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list