Modernize some bits of code.

* modernize the handling of errors
* rename some variables as they use reserved words
* fix some typos and deprecated use of parameters

5 files changed, 79 insertions, 95 deletions

diff --git a/src/msec/config.py b/src/msec/config.py
index 0da2389..8f61602 100755
--- a/src/msec/config.py
+++ b/src/msec/config.py
@@ -87,7 +87,7 @@ SYS_ENC = sys.getfilesystemencoding()
 # localized help
 try:
     from help import HELP
-except:
+except ImportError:
     HELP = {}
 
 # helper function to find documentation for an option
@@ -216,8 +216,8 @@ class MsecConfig:
             return True
         try:
             fd = open(self.config)
-        except:
-            self.log.error(_("Unable to load configuration file %s: %s") % (self.config, sys.exc_info()[1]))
+        except OSError as e:
+            self.log.error(_("Unable to load configuration file %s: %s") % (self.config, e))
             return False
         for line in fd.readlines():
             line = line.strip()
@@ -230,7 +230,7 @@ class MsecConfig:
             try:
                 option, val = line.split("=", 1)
                 self.options[option] = val
-            except:
+            except ValueError:
                 self.log.warn(_("Bad config option: %s") % line)
                 continue
         fd.close()
@@ -265,8 +265,8 @@ class MsecConfig:
             return True
         try:
             fd = open(self.config, "w")
-        except:
-            self.log.error(_("Unable to save %s: %s") % (self.config, sys.exc_info()[1]))
+        except OSError as e:
+            self.log.error(_("Unable to save %s: %s") % (self.config, e))
             return False
         for comment in self.comments:
             print(comment, file=fd)
@@ -324,9 +324,9 @@ class ExceptionConfig:
             return True
         try:
             fd = open(self.config)
-        except:
+        except OSError as e:
             # this file is optional, so if it is not found that's not fatal
-            self.log.info(_("loading exceptions file %s: %s") % (self.config, sys.exc_info()[1]))
+            self.log.info(_("loading exceptions file %s: %s") % (self.config, e))
             self.log.info(_("No exceptions loaded"))
             return False
         for line in fd.readlines():
@@ -340,7 +340,7 @@ class ExceptionConfig:
             try:
                 option, val = line.split(" ", 1)
                 self.options.append((option, val))
-            except:
+            except ValueError:
                 self.log.warn(_("Bad config option: %s") % line)
                 continue
         fd.close()
@@ -379,8 +379,8 @@ class ExceptionConfig:
             return True
         try:
             fd = open(self.config, "w")
-        except:
-            self.log.error(_("Unable to save %s: %s") % (self.config, sys.exc_info()[1]))
+        except OSError as e:
+            self.log.error(_("Unable to save %s: %s") % (self.config, e))
             return False
         for comment in self.comments:
             print(comment, file=fd)
@@ -430,8 +430,8 @@ class PermConfig(MsecConfig):
         """Loads and parses configuration file"""
         try:
             fd = open(self.config)
-        except:
-            self.log.error(_("Unable to load configuration file %s: %s") % (self.config, sys.exc_info()[1]))
+        except OSError as e:
+            self.log.error(_("Unable to load configuration file %s: %s") % (self.config, e))
             return False
         # Look up acl pattern: user1:rwx,user2:x
         acl_re = re.compile(r'^[a-z][a-z0-9_-]+:(?:[rwx]{1,3})(?:,[a-z][a-z0-9_-]+:[rwx]{1,3})*$')
@@ -448,7 +448,6 @@ class PermConfig(MsecConfig):
                 if res:
                     if len(res[0]) == 6:
                         file, user, group, perm, force, acl = res[0]
-
                         # validate force field
                         if force not in ('force', ''):
                             self.log.warn(_("Invalid force value '%s' for '%s', ignoring") % (force, file))
@@ -469,7 +468,7 @@ class PermConfig(MsecConfig):
                         self.options_order.append(file)
                     else:
                         self.log.warn(_("Unexpected format in line: %s") % line)
-            except:
+            except Exception as e:
                 traceback.print_exc()
                 self.log.warn(_("Bad config option: %s") % line)
                 continue
@@ -501,8 +500,8 @@ class PermConfig(MsecConfig):
         """Saves configuration. Comments go on top. If a variable is present in base_level, and it is identical to the one to be saved, it is skipped"""
         try:
             fd = open(self.config, "w")
-        except:
-            self.log.error(_("Unable to save %s: %s") % (self.config, sys.exc_info()[1]))
+        except OSError as e:
+            self.log.error(_("Unable to save %s: %s") % (self.config, e))
             return False
         for comment in self.comments:
             print(comment, file=fd)
diff --git a/src/msec/libmsec.py b/src/msec/libmsec.py
index a683558..a1aeb32 100755
--- a/src/msec/libmsec.py
+++ b/src/msec/libmsec.py
@@ -79,8 +79,8 @@ def move(old, new):
         pass
     try:
         os.rename(old, new)
-    except:
-        error('rename %s %s: %s' % (old, new, str(sys.exc_info()[1])))
+    except Exception as e:
+        error('rename %s %s: %s' % (old, new, e))
 
 def substitute_re_result(res, s):
     for idx in range(0, (res.lastindex or 0) + 1):
@@ -121,8 +121,8 @@ class Log:
                 formatter = logging.Formatter('%(name)s: %(levelname)s: %(message)s')
                 self.syslog_h.setFormatter(formatter)
                 self.logger.addHandler(self.syslog_h)
-            except:
-                print("Logging to syslog not available: %s" % (sys.exc_info()[1]), file=sys.stderr)
+            except Exception as e:
+                print("Logging to syslog not available: %s" % e, file=sys.stderr)
                 interactive = True
 
         # log to file
@@ -132,8 +132,8 @@ class Log:
                 formatter = logging.Formatter('%(asctime)s %(levelname)s: %(message)s')
                 self.file_h.setFormatter(formatter)
                 self.logger.addHandler(self.file_h)
-            except:
-                print("Logging to '%s' not available: %s" % (self.log_path, sys.exc_info()[1]), file=sys.stderr)
+            except Exception as e:
+                print("Logging to '%s' not available: %s" % (self.log_path, e), file=sys.stderr)
                 interactive = True
 
         # interactive logging
@@ -149,7 +149,7 @@ class Log:
         """Attempts to decode a unicode message"""
         try:
             msg = message.decode('UTF-*')
-        except:
+        except Exception:
             msg = message
         return msg
 
@@ -371,21 +371,21 @@ class ConfigFile:
             if self.exists():
                 try:
                     os.unlink(self.path)
-                except:
-                    os.error('unlink %s: %s' % (self.path, str(sys.exc_info()[1])))
+                except OSError as e:
+                    os.error('unlink %s: %s' % (self.path, e))
                 self.log.info(_('deleted %s') % (self.path,))
         elif self.is_touched:
             if os.path.exists(self.path):
                 try:
                     os.utime(self.path, None)
-                except:
-                    self.log.error('utime %s: %s' % (self.path, str(sys.exc_info()[1])))
+                except OSError as e:
+                    self.log.error('utime %s: %s' % (self.path, e))
             elif self.suffix and os.path.exists(self.path + self.suffix):
                 move(self.path + self.suffix, self.path)
                 try:
                     os.utime(self.path, None)
-                except:
-                    self.log.error('utime %s: %s' % (self.pathN, str(sys.exc_info()[1])))
+                except OSError as e:
+                    self.log.error('utime %s: %s' % (self.path, e))
             else:
                 self.lines = []
                 self.is_modified = 1
@@ -403,14 +403,14 @@ class ConfigFile:
                if not done:
                    try:
                        os.unlink(self.path)
-                   except:
-                       self.log.error('unlink %s: %s' % (self.path, str(sys.exc_info()[1])))
+                   except OSError as e:
+                       self.log.error('unlink %s: %s' % (self.path, e))
                    self.log.info(_('deleted %s') % (self.path,))
             if not done:
                 try:
                     os.symlink(self.sym_link, self.path)
-                except:
-                    self.log.error('symlink %s %s: %s' % (self.sym_link, self.path, str(sys.exc_info()[1])))
+                except OSError as e:
+                    self.log.error('symlink %s %s: %s' % (self.sym_link, self.path, e))
                 self.log.info(_('made symbolic link from %s to %s') % (self.sym_link, self.path))
         elif self.is_moved:
             move(self.path, self.path + self.suffix)
@@ -681,8 +681,8 @@ class MSEC:
                     plugin = plugin_class(log=self.log, configfiles=self.configfiles, root=self.root)
                     self.plugins[plugin_name] = plugin
                     self.log.debug("Loaded plugin '%s'" % plugin_f)
-                except:
-                    self.log.error(_("Error loading plugin '%s' from %s: %s") % (plugin_f, file, sys.exc_info()[1]))
+                except Exception as e:
+                    self.log.error(_("Error loading plugin '%s' from %s: %s") % (plugin_f, file, e))
 
     def reset(self):
         """Resets the configuration"""
@@ -697,7 +697,7 @@ class MSEC:
         # finding out what function to call
         try:
             plugin_, callback = name.split(".", 1)
-        except:
+        except ValueError:
             # bad format?
             self.log.error(_("Invalid callback: %s") % (name))
             return None
@@ -869,8 +869,8 @@ class PERMS:
                     self.log.warn(_("Forcing ownership of %s to %s") % (file, self.get_user_name(newuser)))
                     try:
                         os.chown(file, newuser, -1)
-                    except:
-                        self.log.error(_("Error changing user on %s: %s") % (file, sys.exc_info()[1]))
+                    except OSError as e:
+                        self.log.error(_("Error changing user on %s: %s") % (file, e))
                 else:
                     self.log.warn(_("Wrong owner of %s: should be %s") % (file, self.get_user_name(newuser)))
             if newgroup != None:
@@ -878,8 +878,8 @@ class PERMS:
                     self.log.warn(_("Enforcing group on %s to %s") % (file, self.get_group_name(newgroup)))
                     try:
                         os.chown(file, -1, newgroup)
-                    except:
-                        self.log.error(_("Error changing group on %s: %s") % (file, sys.exc_info()[1]))
+                    except OSError as e:
+                        self.log.error(_("Error changing group on %s: %s") % (file, e))
                 else:
                     self.log.warn(_("Wrong group of %s: should be %s") % (file, self.get_group_name(newgroup)))
             # permissions should be last, as chown resets them
@@ -889,8 +889,8 @@ class PERMS:
                     self.log.warn(_("Enforcing permissions on %s to %o") % (file, newperm))
                     try:
                         os.chmod(file, newperm)
-                    except:
-                        self.log.error(_("Error changing permissions on %s: %s") % (file, sys.exc_info()[1]))
+                    except OSError as e:
+                        self.log.error(_("Error changing permissions on %s: %s") % (file, e))
                 else:
                     self.log.warn(_("Wrong permissions of %s: should be %o") % (file, newperm))
 
@@ -911,8 +911,8 @@ class PERMS:
                             if ret != 0:
                                 # problem setting setfacl
                                 self.log.error(_("Unable to add filesystem-specific ACL %s to %s") % (acluser, file))
-                    except:
-                        self.log.error(_("Error changing acl on %s: %s") % (file, sys.exc_info()[1]))
+                    except Exception as e:
+                        self.log.error(_("Error changing acl on %s: %s") % (file, e))
                 else:
                     self.log.warn(_("Wrong acl of %s") % (file))
 
diff --git a/src/msec/msec.py b/src/msec/msec.py
index d4f366b..aac8e82 100755
--- a/src/msec/msec.py
+++ b/src/msec/msec.py
@@ -6,10 +6,9 @@ and works as a frontend to libmsec.
 
 import sys
 import os
-import string
 import getopt
 import gettext
-import re
+import logging
 
 # config
 import config
@@ -17,14 +16,12 @@ import config
 # version
 try:
     from version import version
-except:
+except ImportError:
     version = "development version"
 
 # libmsec
 from libmsec import MSEC, Log
 
-import logging
-
 # localization
 try:
     gettext.install('msec')
diff --git a/src/msec/msecgui.py b/src/msec/msecgui.py
index cc20452..3b0aafb 100755
--- a/src/msec/msecgui.py
+++ b/src/msec/msecgui.py
@@ -5,8 +5,8 @@ This is graphical frontend to msec.
 
 import os
 import sys
-import string
 import getopt
+import logging
 import signal
 import traceback
 import queue
@@ -47,8 +47,6 @@ except:
 # libmsec
 from libmsec import MSEC, PERMS, Log
 
-import logging
-
 # localization
 import gettext
 try:
@@ -142,8 +140,8 @@ class BackgroundRunner(Thread):
         try:
             res = os.system(self.program)
             self.finish.put(res)
-        except:
-            print("Aborted: %s" % sys.exc_info()[1])
+        except Exception as e:
+            print("Aborted: %s" % e)
             self.finish.put(-1)
 
 class MsecGui(Gtk.Window):
@@ -233,7 +231,6 @@ class MsecGui(Gtk.Window):
             menubar.add(filemenu)
             menu = Gtk.Menu()
             filemenu.set_submenu(menu)
-            group = None
             for submenu, callback in items:
                 menuitem = Gtk.MenuItem(label=submenu, use_underline=True)
                 #menuitem = Gtk.MenuItem(submenu)
@@ -319,9 +316,9 @@ class MsecGui(Gtk.Window):
         # check for changed options
         num_changes = 0
         changes = []
-        for name, type, oldconf, curconf in [ (_("MSEC option changes"), _("option"), self.oldconfig, curconfig),
-                                        (_("System permissions changes"), _("permission check"), self.oldperms, curperms),
-                                        ]:
+        for name, change_type, oldconf, curconf in [
+            (_("MSEC option changes"),        _("option"),           self.oldconfig, curconfig),
+            (_("System permissions changes"), _("permission check"), self.oldperms, curperms), ]:
             # check for changes
             opt_changes = []
             opt_adds = []
@@ -330,17 +327,17 @@ class MsecGui(Gtk.Window):
             curchanges = ""
             opt_changes = [opt for opt in oldconf if (curconf.get(opt) != oldconf.get(opt) and curconf.get(opt) != None and curconf.get(opt) != None)]
             if len(opt_changes) > 0:
-                curchanges += "\n\t" + "\n\t".join([_("changed %s <b>%s</b> (%s -> %s)") % (type, param, oldconf.get(param), curconf.get(param)) for param in opt_changes])
+                curchanges += "\n\t" + "\n\t".join([_("changed %s <b>%s</b> (%s -> %s)") % (change_type, param, oldconf.get(param), curconf.get(param)) for param in opt_changes])
                 num_changes += len(opt_changes)
             # new options
             opt_adds = [opt for opt in curconf.list_options() if (opt not in oldconf and curconf.get(opt))]
             if len(opt_adds) > 0:
-                curchanges += "\n\t" + "\n\t".join([_("added %s <b>%s</b> (%s)") % (type, param, curconf.get(param)) for param in opt_adds])
+                curchanges += "\n\t" + "\n\t".join([_("added %s <b>%s</b> (%s)") % (change_type, param, curconf.get(param)) for param in opt_adds])
                 num_changes += len(opt_adds)
             # removed options
             opt_dels = [opt for opt in oldconf if ((opt not in curconf.list_options() or curconf.get(opt) == None) and oldconf.get(opt))]
             if len(opt_dels) > 0:
-                curchanges += "\n\t" + "\n\t".join([_("removed %s <b>%s</b>") % (type, param) for param in opt_dels])
+                curchanges += "\n\t" + "\n\t".join([_("removed %s <b>%s</b>") % (change_type, param) for param in opt_dels])
                 num_changes += len(opt_dels)
             # adding labels
             if curchanges == "":
@@ -379,7 +376,6 @@ class MsecGui(Gtk.Window):
         sw.set_policy(Gtk.PolicyType.NEVER, Gtk.PolicyType.AUTOMATIC)
         exp_vbox.pack_start(sw, True, True, DEFAULT_SPACING)
 
-
         vbox = VBox()
         exp_vbox.set_size_request(640, 280)
         sw.add(vbox)
@@ -415,12 +411,10 @@ class MsecGui(Gtk.Window):
             textview.set_wrap_mode(Gtk.WrapMode.WORD_CHAR)
             textview.set_editable(False)
             expander.add(textview)
-            count = 1
-            for msg in msgs:
-                buffer = textview.get_buffer()
-                end = buffer.get_end_iter()
-                buffer.insert(end, "%d: %s\n" % (count, msg))
-                count += 1
+            text_buffer = textview.get_buffer()
+            for count, msg in enumerate(msgs, start=1):
+                end = text_buffer.get_end_iter()
+                text_buffer.insert(end, "%d: %s\n" % (count, msg))
             vbox_advanced.pack_start(expander, False, False, padding=DEFAULT_SPACING)
 
         # hide all information in an expander
@@ -609,8 +603,8 @@ class MsecGui(Gtk.Window):
                     image.set_from_pixbuf(pixbuf)
                     image.set_hexpand(False)
                     grid.attach(image, 0, row, 1, 1)
-                except:
-                    print("Unable to load icon %s: %s" % (icon, sys.exc_info()[1]))
+                except Exception as e:
+                    print("Unable to load icon %s: %s" % (icon, e))
 
             grid.attach(make_status_label(text), 1, row, 1, 1)
 
@@ -640,9 +634,9 @@ class MsecGui(Gtk.Window):
         if baselevel == config.NONE_LEVEL:
             msec_status = [_("Msec is disabled")]
         else:
-            msec_status = []
-            msec_status.append(_("Msec is enabled"))
-            msec_status.append(_("Base security level: '%s'") % baselevel)
+            msec_status = [_("Msec is enabled"),
+                           _("Base security level: '%s'") % baselevel
+            ]
             # find out custom settings
             custom_count = 0
             base_config = self.msec_defaults[baselevel]
@@ -704,8 +698,8 @@ class MsecGui(Gtk.Window):
         try:
             with open(logfile, "r") as fd:
                 data = fd.readlines()
-        except:
-            data = [_("Unable to read log file: %s") % sys.exc_info()[1]]
+        except OSError as e:
+            data = [_("Unable to read log file: %s") % e]
 
         dialog = Gtk.Dialog(title=_("Periodic check results"),
                             transient_for=self.window, modal=True)
@@ -734,7 +728,7 @@ class MsecGui(Gtk.Window):
         """Shows results for the test"""
         dialog = Gtk.MessageDialog(
                 transient_for=self.window,
-                flags=0,
+                modal=True,
                 message_type=Gtk.MessageType.INFO,
                 buttons=Gtk.ButtonsType.YES_NO)
         dialog.set_markup(_("Do you want to run the <b>%s</b> periodic check? Please note that it could take a considerable time to finish.") % check)
@@ -743,6 +737,7 @@ class MsecGui(Gtk.Window):
         dialog.destroy()
         if ret != Gtk.ResponseType.YES:
             return
+
         # progress bar
         progress = Gtk.Window()
         progress.set_title(_("Please wait, running checks..."))
@@ -767,7 +762,7 @@ class MsecGui(Gtk.Window):
         q = queue.Queue()
 
         if check == "manual":
-            program = "/usr/share/msec/security.sh"
+            program = os.path.join(config.MSEC_DIR, "security.sh")
         else:
             program = "/etc/cron.%s/msec" % check
         installer = BackgroundRunner(finish=q, program=program)
@@ -786,15 +781,15 @@ class MsecGui(Gtk.Window):
 
         if result == 0:
             text = _("Periodic check was executed successfully!")
-            type = Gtk.MessageType.INFO
+            msg_type = Gtk.MessageType.INFO
         else:
             text = _("An error occurred while running periodic check.")
-            type = Gtk.MessageType.ERROR
+            msg_type = Gtk.MessageType.ERROR
         # policy was initialized
         dialog = Gtk.MessageDialog(
                 transient_for=self.window,
-                flags=0,
-                message_type=type,
+                modal=True,
+                message_type=msg_type,
                 text=text,
                 buttons=Gtk.ButtonsType.OK
                 )
@@ -1048,7 +1043,6 @@ class MsecGui(Gtk.Window):
                     iter = options.iter_next(iter)
             elif curconfig.__class__ == config.PermConfig:
                 self.reset_permissions(None, options, level=level)
-                pass
             else:
                 #print curconfig.__class__
                 pass
@@ -1289,10 +1283,7 @@ class MsecGui(Gtk.Window):
             user_s, group_s, perm_s, force, acl = self.permconfig.get(file)
 
             # convert to boolean
-            if force:
-                force = True
-            else:
-                force = False
+            force = bool(force)
 
             # building the option
             iter = lstore.append()
@@ -1356,10 +1347,7 @@ class MsecGui(Gtk.Window):
             user_s, group_s, perm_s, force_s, acls = defperms.get(file)
 
             # convert to boolean
-            if force_s:
-                force_val = True
-            else:
-                force_val = False
+            force_val = bool(force_s)
 
             # building the option
             iter = model.append()
diff --git a/src/msec/tools.py b/src/msec/tools.py
index 50b9208..109914c 100755
--- a/src/msec/tools.py
+++ b/src/msec/tools.py
@@ -31,8 +31,8 @@ def find_firewall_info(log):
         for l in data.splitlines():
             if l[:3] == "-A ":
                 firewall_entries.append(l.strip())
-    except:
-        log.error(_("Unable to parse firewall configuration: %s") % sys.exc_info()[1])
+    except OSError as e:
+        log.error(_("Unable to parse firewall configuration: %s") % e)
     # not find out if the firewall is enabled
     if len(firewall_entries) == 0:
         firewall_status = _("Disabled")
@@ -48,8 +48,8 @@ def get_updates_status(log, updatedir="/var/lib/urpmi"):
         updated = time.localtime(ret[stat.ST_MTIME])
         updated_s = time.strftime(locale.nl_langinfo(locale.D_T_FMT), updated)
         status = _("Last updated: %s") % updated_s
-    except:
-        log.error(_("Unable to access %s: %s") % (updatedir, sys.exc_info()[1]))
+    except OSError as e:
+        log.error(_("Unable to access %s: %s") % (updatedir, e))
         status = _("Unable to determine update status")
     return status