On 26/09/2014 17:43, thierry bordaz wrote:
Hello,

    When called from set_selinux_booleans, if not selinux_enabled, you
    may want to 'return False' rather than 'return'.
    Now it looks like callers of set_selinux_booleans do not check the
    returned value :-)

    thanks
    thierry

On 09/26/2014 05:26 PM, Francesco Marella wrote:
This should be the final one.

fm

On 26/09/2014 16:30, Francesco Marella wrote:

On 26/09/2014 15:41, Petr Viktorin wrote:

Hello! Thanks for the patch!

The new function is not one of the platform-independent tasks, and doesn't even use `self`, so you can define it as a module-level helper function.

But more importantly, this won't work: the blocks you are replacing return from their functions. You'd need to use something like:
    if not selinux_enabled():
        return
instead of:
    self.check_enabled_selinux()




_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel



_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


From 5463ce197addb741ff728be2f2f11609b71e9565 Mon Sep 17 00:00:00 2001
From: Francesco Marella <fmare...@gmx.com>
Date: Fri, 26 Sep 2014 14:07:25 +0200
Subject: [PATCH] Refactor selinuxenabled check

Ticket: https://fedorahosted.org/freeipa/ticket/4571
---
 ipaplatform/fedora/tasks.py | 43 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/ipaplatform/fedora/tasks.py b/ipaplatform/fedora/tasks.py
index 9f4a76b8208cc78c330dc022730c4faac09995f9..d1d4abb97d84404cd65ade9ed411a6d1503c9125 100644
--- a/ipaplatform/fedora/tasks.py
+++ b/ipaplatform/fedora/tasks.py
@@ -48,6 +48,22 @@ from ipaplatform.base.tasks import BaseTaskNamespace
 log = log_mgr.get_logger(__name__)
 
 
+def selinux_enabled():
+    """
+    Check if SELinux is enabled.
+    """
+    if os.path.exists(paths.SELINUXENABLED):
+        try:
+            ipautil.run([paths.SELINUXENABLED])
+            return True
+        except ipautil.CalledProcessError:
+            # selinuxenabled returns 1 if not enabled
+            return False
+    else:
+        # No selinuxenabled, no SELinux
+        return False
+
+
 class FedoraTaskNamespace(BaseTaskNamespace):
 
     def restore_context(self, filepath, restorecon=paths.SBIN_RESTORECON):
@@ -59,14 +75,8 @@ class FedoraTaskNamespace(BaseTaskNamespace):
 
         ipautil.run() will do the logging.
         """
-        try:
-            if os.path.exists(paths.SELINUXENABLED):
-                ipautil.run([paths.SELINUXENABLED])
-            else:
-                # No selinuxenabled, no SELinux
-                return
-        except ipautil.CalledProcessError:
-            # selinuxenabled returns 1 if not enabled
+
+        if not selinux_enabled():
             return
 
         if (os.path.exists(restorecon)):
@@ -82,14 +92,7 @@ class FedoraTaskNamespace(BaseTaskNamespace):
         This function returns nothing but may raise a Runtime exception
         if SELinux is enabled but restorecon is not available.
         """
-        try:
-            if os.path.exists(paths.SELINUXENABLED):
-                ipautil.run([paths.SELINUXENABLED])
-            else:
-                # No selinuxenabled, no SELinux
-                return
-        except ipautil.CalledProcessError:
-            # selinuxenabled returns 1 if not enabled
+        if not selinux_enabled():
             return
 
         if not os.path.exists(restorecon):
@@ -336,13 +339,7 @@ class FedoraTaskNamespace(BaseTaskNamespace):
 
             return args
 
-        if (os.path.exists(paths.SELINUXENABLED)):
-            try:
-                ipautil.run([paths.SELINUXENABLED])
-            except ipautil.CalledProcessError:
-                # selinuxenabled returns 1 if not enabled
-                return False
-        else:
+        if not selinux_enabled():
             return False
 
         updated_vars = {}
-- 
2.1.1

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to