[libvirt] [PATCH] maint: cleanup detection of const'ness of selinux ctx
Commit 292d3f2d fixed the build with libselinux 2.3, but missed some suggestions by eblake https://www.redhat.com/archives/libvir-list/2014-May/msg00977.html This patch changes the macro introduced in 292d3f2d to either be empty in the case of newer libselinux, or contain 'const' in the case of older libselinux. The macro is then used directly in tests/securityselinuxhelper.c. --- m4/virt-selinux.m4 | 7 +++++-- tests/securityselinuxhelper.c | 24 ++++-------------------- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/m4/virt-selinux.m4 b/m4/virt-selinux.m4 index 1d899d5..a6f89ba 100644 --- a/m4/virt-selinux.m4 +++ b/m4/virt-selinux.m4 @@ -39,8 +39,11 @@ int setcon(const security_context_t context); [gt_cv_setcon_param='security_context_t'], [gt_cv_setcon_param='const char*'])]) if test "$gt_cv_setcon_param" = 'const char*'; then - AC_DEFINE_UNQUOTED([SELINUX_CTX_CHAR_PTR], 1, - [SELinux uses newer char * for security context]) + AC_DEFINE([VIR_SELINUX_CTX_CONST], [const], + [SELinux uses newer const char * for security context]) + else + AC_DEFINE([VIR_SELINUX_CTX_CONST], [], + [SELinux uses newer const char * for security context]) fi AC_MSG_CHECKING([SELinux mount point]) diff --git a/tests/securityselinuxhelper.c b/tests/securityselinuxhelper.c index af4fae4..1252c15 100644 --- a/tests/securityselinuxhelper.c +++ b/tests/securityselinuxhelper.c @@ -156,11 +156,7 @@ int getpidcon(pid_t pid, security_context_t *context) return getpidcon_raw(pid, context); } -#ifdef SELINUX_CTX_CHAR_PTR -int setcon_raw(const char *context) -#else -int setcon_raw(security_context_t context) -#endif +int setcon_raw(VIR_SELINUX_CTX_CONST char *context) { if (!is_selinux_enabled()) { errno = EINVAL; @@ -169,21 +165,13 @@ int setcon_raw(security_context_t context) return setenv("FAKE_SELINUX_CONTEXT", context, 1); } -#ifdef SELINUX_CTX_CHAR_PTR -int setcon(const char *context) -#else -int setcon(security_context_t context) -#endif +int setcon(VIR_SELINUX_CTX_CONST char *context) { return setcon_raw(context); } -#ifdef SELINUX_CTX_CHAR_PTR -int setfilecon_raw(const char *path, const char *con) -#else -int setfilecon_raw(const char *path, security_context_t con) -#endif +int setfilecon_raw(const char *path, VIR_SELINUX_CTX_CONST char *con) { const char *constr = con; if (STRPREFIX(path, abs_builddir "/securityselinuxlabeldata/nfs/")) { @@ -194,11 +182,7 @@ int setfilecon_raw(const char *path, security_context_t con) constr, strlen(constr), 0); } -#ifdef SELINUX_CTX_CHAR_PTR -int setfilecon(const char *path, const char *con) -#else -int setfilecon(const char *path, security_context_t con) -#endif +int setfilecon(const char *path, VIR_SELINUX_CTX_CONST char *con) { return setfilecon_raw(path, con); } -- 1.7.9.2
On 05/28/2014 01:54 PM, Jim Fehlig wrote:
Commit 292d3f2d fixed the build with libselinux 2.3, but missed some suggestions by eblake
https://www.redhat.com/archives/libvir-list/2014-May/msg00977.html
This patch changes the macro introduced in 292d3f2d to either be empty in the case of newer libselinux, or contain 'const' in the case of older libselinux. The macro is then used directly in tests/securityselinuxhelper.c. --- m4/virt-selinux.m4 | 7 +++++-- tests/securityselinuxhelper.c | 24 ++++-------------------- 2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/m4/virt-selinux.m4 b/m4/virt-selinux.m4 index 1d899d5..a6f89ba 100644 --- a/m4/virt-selinux.m4 +++ b/m4/virt-selinux.m4 @@ -39,8 +39,11 @@ int setcon(const security_context_t context); [gt_cv_setcon_param='security_context_t'], [gt_cv_setcon_param='const char*'])]) if test "$gt_cv_setcon_param" = 'const char*'; then - AC_DEFINE_UNQUOTED([SELINUX_CTX_CHAR_PTR], 1, - [SELinux uses newer char * for security context]) + AC_DEFINE([VIR_SELINUX_CTX_CONST], [const], + [SELinux uses newer const char * for security context]) + else + AC_DEFINE([VIR_SELINUX_CTX_CONST], [], + [SELinux uses newer const char * for security context]) fi
That feels complex to have two competing AC_DEFINE. By using a single AC_DEFINE_UNQUOTED and judicious contents of the _cv_ shell variable, you can get by with one. Oh, and while we are at it, we should be using the libvirt prefix of lv_, not the gettext prefix of gt_.
-#ifdef SELINUX_CTX_CHAR_PTR -int setcon_raw(const char *context) -#else -int setcon_raw(security_context_t context)
[Note that setcon_raw(security_context_t context) and setcon_raw(const security_context_t context) are compatible; the compiler treats 'const typedef_to_pointer' the same as 'type *const' (a pointer that can't be changed once initialized, but whose contents can be altered at will) and NOT as 'const type *' (a pointer that can be changed at will, but where the contents of the current pointer value cannot be changed). That's probably why newer libselinux ditched the typedef and went with a verbatim 'const char *', even though it was not compile-time back-compatible.]
-#endif +int setcon_raw(VIR_SELINUX_CTX_CONST char *context)
So it actually works for libselinx 2.3 (I'm assuming your testing covered it) and 2.2 (I tested that). Okay, it is indeed nicer. ACK with this squashed in, if it still passes for you: diff --git i/m4/virt-selinux.m4 w/m4/virt-selinux.m4 index a6f89ba..357b758 100644 --- i/m4/virt-selinux.m4 +++ w/m4/virt-selinux.m4 @@ -29,22 +29,18 @@ AC_DEFUN([LIBVIRT_CHECK_SELINUX],[ if test "$with_selinux" = "yes"; then # libselinux changed signatures between 2.2 and 2.3 - AC_CACHE_CHECK([for selinux setcon parameter type], [gt_cv_setcon_param], + AC_CACHE_CHECK([for selinux setcon parameter type], [lv_cv_setcon_const], [AC_COMPILE_IFELSE( [AC_LANG_PROGRAM( [[ #include <selinux/selinux.h> -int setcon(const security_context_t context); +int setcon(char *context); ]])], - [gt_cv_setcon_param='security_context_t'], - [gt_cv_setcon_param='const char*'])]) - if test "$gt_cv_setcon_param" = 'const char*'; then - AC_DEFINE([VIR_SELINUX_CTX_CONST], [const], - [SELinux uses newer const char * for security context]) - else - AC_DEFINE([VIR_SELINUX_CTX_CONST], [], - [SELinux uses newer const char * for security context]) - fi + [lv_cv_setcon_const=''], + [lv_cv_setcon_const='const'])]) + AC_DEFINE_UNQUOTED([VIR_SELINUX_CTX_CONST], [$lv_cv_setcon_const], + [Define to empty or 'const' depending on how SELinux qualifies its + security context parameters]) AC_MSG_CHECKING([SELinux mount point]) if test "$with_selinux_mount" = "check" || test -z "$with_selinux_mount"; then -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Eric Blake wrote:
On 05/28/2014 01:54 PM, Jim Fehlig wrote:
Commit 292d3f2d fixed the build with libselinux 2.3, but missed some suggestions by eblake
https://www.redhat.com/archives/libvir-list/2014-May/msg00977.html
This patch changes the macro introduced in 292d3f2d to either be empty in the case of newer libselinux, or contain 'const' in the case of older libselinux. The macro is then used directly in tests/securityselinuxhelper.c. --- m4/virt-selinux.m4 | 7 +++++-- tests/securityselinuxhelper.c | 24 ++++-------------------- 2 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/m4/virt-selinux.m4 b/m4/virt-selinux.m4 index 1d899d5..a6f89ba 100644 --- a/m4/virt-selinux.m4 +++ b/m4/virt-selinux.m4 @@ -39,8 +39,11 @@ int setcon(const security_context_t context); [gt_cv_setcon_param='security_context_t'], [gt_cv_setcon_param='const char*'])]) if test "$gt_cv_setcon_param" = 'const char*'; then - AC_DEFINE_UNQUOTED([SELINUX_CTX_CHAR_PTR], 1, - [SELinux uses newer char * for security context]) + AC_DEFINE([VIR_SELINUX_CTX_CONST], [const], + [SELinux uses newer const char * for security context]) + else + AC_DEFINE([VIR_SELINUX_CTX_CONST], [], + [SELinux uses newer const char * for security context]) fi
That feels complex to have two competing AC_DEFINE. By using a single AC_DEFINE_UNQUOTED and judicious contents of the _cv_ shell variable, you can get by with one. Oh, and while we are at it, we should be using the libvirt prefix of lv_, not the gettext prefix of gt_.
-#ifdef SELINUX_CTX_CHAR_PTR -int setcon_raw(const char *context) -#else -int setcon_raw(security_context_t context)
[Note that setcon_raw(security_context_t context) and setcon_raw(const security_context_t context) are compatible; the compiler treats 'const typedef_to_pointer' the same as 'type *const' (a pointer that can't be changed once initialized, but whose contents can be altered at will) and NOT as 'const type *' (a pointer that can be changed at will, but where the contents of the current pointer value cannot be changed). That's probably why newer libselinux ditched the typedef and went with a verbatim 'const char *', even though it was not compile-time back-compatible.]
-#endif +int setcon_raw(VIR_SELINUX_CTX_CONST char *context)
So it actually works for libselinx 2.3 (I'm assuming your testing covered it) and 2.2 (I tested that). Okay, it is indeed nicer. ACK with this squashed in, if it still passes for you:
Still passes my tests, which include libselinux 2.1, 2.2, and 2.3. Pushed now. Thanks for the help! Regards, Jim
participants (2)
-
Eric Blake -
Jim Fehlig