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()


From d7422bb4de03219ec8e8326b04bde4c5e7fe4b02 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 | 44 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/ipaplatform/fedora/tasks.py b/ipaplatform/fedora/tasks.py
index 9f4a76b8208cc78c330dc022730c4faac09995f9..7387dc920ab04023dfa8a79fc417ff9091bc0a98 100644
--- a/ipaplatform/fedora/tasks.py
+++ b/ipaplatform/fedora/tasks.py
@@ -48,6 +48,21 @@ 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])
+        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 +74,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 +91,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,14 +338,8 @@ 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:
-            return False
+        if not selinux_enabled():
+            return
 
         updated_vars = {}
         failed_vars = {}
-- 
2.1.1

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

Reply via email to