On 03/27/2013 10:49 AM, Jakub Hrozek wrote:
On Tue, Mar 26, 2013 at 03:12:24PM +0100, Ondrej Kos wrote:
> Thanks for the review, new patch is attached
>
> Ondra
>
> --
> Ondrej Kos
> Associate Software Engineer
> Identity Management
> Red Hat Czech
>
> phone: +420-532-294-558
> cell: +420-736-417-909
> ext: 82-62558
> loc: 1013 Brno 1 office
> irc: okos @ #sssd #brno
I can't compile the patch with libnl1:
src/monitor/monitor_netlink.c: In function ‘setup_netlink’:
src/monitor/monitor_netlink.c:753:9: error: too many arguments to function ‘nl_geterror’
In file included from src/monitor/monitor_netlink.c:44:0:
/usr/include/netlink/utils.h:41:15: note: declared here
make[2]: *** [src/monitor/monitor_netlink.o] Error 1
$ rpm -qa \*libnl\*
libnl-devel-1.1-14.el6.x86_64
libnl-1.1-14.el6.x86_64
libnl-debuginfo-1.1-14.el6.x86_64
$ grep -i libnl config.h
#define HAVE_LIBNL 1
#define HAVE_LIBNL1 1
/* #undef HAVE_LIBNL3 */
/* Does libnl have nl_set_passcred? */
/* Does libnl have nl_socket_add_membership? */
/* Does libnl have nl_socket_modify_cb? */
/* Does libnl have rtnl_route_get_oif? */
> diff --git a/src/external/libnl.m4 b/src/external/libnl.m4
> index
36619ff87490fa94651255224b9fb57e3194ecbc..9bc6f43e86d14f098e8419280e9d374ef443b73d 100644
> --- a/src/external/libnl.m4
> +++ b/src/external/libnl.m4
> @@ -1,34 +1,85 @@
> +dnl A macro to check if this particular version of libnl supports particular common
libnl functions
> +AC_DEFUN([AM_CHECK_LIBNL_FCS],
> +[
> + if test x$1 != x; then
> +
Why do you check for the presence of the parameter here? If it's not
set, then the function is a noop, is that expected?
If there is a use-case for a blank $1, then I think m4_ifval or
m4_default are better choice.
this was just a bit of defensive coding, i didn't want to accidentally
check against empty parameter.
> + AC_CHECK_LIB($1,
> + [nl_socket_add_membership],
> + [AC_DEFINE([HAVE_NL_SOCKET_ADD_MEMBERSHIP], 1, [Does libnl have
nl_socket_add_membership?])
> + ],
> + )
> +
> + AC_CHECK_LIB($1,
> + [nl_socket_modify_cb],
> + [AC_DEFINE([HAVE_NL_SOCKET_MODIFY_CB], 1, [Does libnl have
nl_socket_modify_cb?])
> + ],
> + )
> +
> + AC_CHECK_LIB($1,
> + [rtnl_route_get_oif],
> + [AC_DEFINE([HAVE_RTNL_ROUTE_GET_OIF], 1, [Does libnl have
rtnl_route_get_oif?])
> + ],
> + )
> +
> + fi
> +])
> +
> dnl A macro to check the availability and version of libnetlink
> +AC_DEFUN([AM_CHECK_LIBNL1],
> +[
> + PKG_CHECK_MODULES(LIBNL, libnl-1 >= 1.1,[
> +
> + HAVE_LIBNL=1
> + HAVE_LIBNL1=1
> +
> + AC_DEFINE_UNQUOTED(HAVE_LIBNL, 1, [Build with libnetlink support])
> + AC_DEFINE_UNQUOTED(HAVE_LIBNL1, 1, [Libnetlink version = 1])
> +
> + AC_MSG_NOTICE([Building with libnl])
>
> -AC_DEFUN([AM_CHECK_LIBNL],
> + AC_CHECK_HEADERS(netlink.h)
> + AC_CHECK_LIB(nl, nl_connect, [ LIBNL_LIBS="-lnl" ],
[AC_MSG_ERROR([libnl is required])])
> +
> + AM_CHECK_LIBNL_FCS(nl)
> +
> + AC_CHECK_LIB($LNL_CHECK_LIB,
> + [nl_set_passcred],
> + [AC_DEFINE([HAVE_NL_SET_PASSCRED], 1, [Does libnl have
nl_set_passcred?])
> + ],
This is not what I meant, sorry. You can move the check to AM_CHECK_LIBNL_FCS
(and the one for nl_socket_set_passcred) and then in the module choose
the one that is available. That would also eliminate the need for
nlw_set_passcred, you would just call the available function directly in
nlw_enable_passcred or return EOK if neither is available.
> + )
> +
> + ],[AC_MSG_WARN([Netlink v1 support unavailable or too old])])
> +
> + AC_SUBST(LIBNL_CFLAGS)
> + AC_SUBST(LIBNL_LIBS)
> +])
> +
> +dnl A macro to check the availability of libnetlink version 3
> +
> +AC_DEFUN([AM_CHECK_LIBNL3],
> [
> - PKG_CHECK_MODULES(libnl, libnl-1 >= 1.1,[
> + PKG_CHECK_MODULES(LIBNL, [
> + libnl-3.0 >= 3.0
> + libnl-route-3.0 >= 3.0], [
> +
> HAVE_LIBNL=1
> - AC_SUBST(HAVE_LIBNL)
> + HAVE_LIBNL3=1
> +
> +
> AC_DEFINE_UNQUOTED(HAVE_LIBNL, 1, [Build with libnetlink support])
> + AC_DEFINE_UNQUOTED(HAVE_LIBNL3, 1, [Libnetlink version = 3])
> +
> + AC_MSG_NOTICE([Building with libnl3])
> +
> + AM_CHECK_LIBNL_FCS(nl-3)
> +
> + AC_CHECK_LIB($LNL_CHECK_LIB,
> + [nl_socket_set_passcred],
> + [AC_DEFINE([HAVE_NL_SET_PASSCRED], 1, [Does libnl have
nl_socket_set_passcred?])
Did you mean:
AC_DEFINE([HAVE_NL_SOCKET_SET_PASSCRED]
?
originally, no. i meant to use the same HAVE_ for both libraries, to
avoid ifdef-elif-else in the main code, but your solution is better.
New patch attached
--
Ondrej Kos
Associate Software Engineer
Identity Management
Red Hat Czech
phone: +420-532-294-558
cell: +420-736-417-909
ext: 82-62558
loc: 1013 Brno 1 office
irc: okos @ #sssd #brno