[Fedora-livecd-list] Patch review request for exception handling

[Bruno Wolff III]

On Sun, Jul 25, 2010 at 02:08:26 -0500,
  Bruno Wolff III <bruno at wolff.to> wrote:
> 
> Going down this road, I have something worked up that would need to be
> tested before committing. The patch is attached.

There is a error in the patch I used 'strerror' instead of 'errno' in one
place. The corrected patch is attached.
-------------- next part --------------
diff --git a/imgcreate/creator.py b/imgcreate/creator.py
index 4deb49e..0ba5af5 100644
--- a/imgcreate/creator.py
+++ b/imgcreate/creator.py
@@ -385,9 +385,9 @@ class ImageCreator(object):
         try:
             self.__builddir = tempfile.mkdtemp(dir =  os.path.abspath(self.tmpdir),
                                                prefix = "imgcreate-")
-        except OSError, (err, msg):
+        except OSError, e:
             raise CreatorError("Failed create build directory in %s: %s" %
-                               (self.tmpdir, msg))
+                               (self.tmpdir, e.strerror))
 
     def __sanity_check(self):
         """Ensure that the config we've been given is sane."""
@@ -705,9 +705,9 @@ class ImageCreator(object):
             try:
                 subprocess.check_call([s.interp, script],
                                       preexec_fn = preexec, env = env)
-            except OSError, (err, msg):
+            except OSError, e:
                 raise CreatorError("Failed to execute %%post script "
-                                   "with '%s' : %s" % (s.interp, msg))
+                                   "with '%s' : %s" % (s.interp, e.strerror))
             except subprocess.CalledProcessError, err:
                 if s.errorOnFail:
                     raise CreatorError("%%post script failed with code %d "
diff --git a/imgcreate/debug.py b/imgcreate/debug.py
index c45047d..84b8d30 100644
--- a/imgcreate/debug.py
+++ b/imgcreate/debug.py
@@ -30,9 +30,9 @@ def handle_logging(option, opt, val, parser, logger, level):
 def handle_logfile(option, opt, val, parser, logger, stream):
     try:
         logfile = logging.FileHandler(val,"a")
-    except IOError, (err, msg):
+    except IOError, e:
         raise optparse.OptionValueError("Cannot open file '%s' : %s" %
-                                        (val, msg))
+                                        (val, e.strerror))
 
 
     logger.removeHandler(stream)
diff --git a/imgcreate/fs.py b/imgcreate/fs.py
index 16445a5..c5cd18b 100644
--- a/imgcreate/fs.py
+++ b/imgcreate/fs.py
@@ -36,8 +36,8 @@ def makedirs(dirname):
     """
     try:
         os.makedirs(dirname)
-    except OSError, (err, msg):
-        if err != errno.EEXIST:
+    except OSError, e:
+        if e.errno != errno.EEXIST:
             raise
 
 def mksquashfs(in_img, out_img, compress_type):
diff --git a/imgcreate/kickstart.py b/imgcreate/kickstart.py
index 328043c..6600fd7 100644
--- a/imgcreate/kickstart.py
+++ b/imgcreate/kickstart.py
@@ -53,9 +53,11 @@ def read_kickstart(path):
     try:
         ksfile = urlgrabber.urlgrab(path)
         ks.readKickstart(ksfile)
-    except IOError, (err, msg):
+# Fallback to e.args[0] is a workaround for bugs in urlgragger and pykickstart.
+    except IOError, e:
         raise errors.KickstartError("Failed to read kickstart file "
-                                    "'%s' : %s" % (path, msg))
+                                    "'%s' : %s" % (path, e.strerror or
+                                    e.args[0]))
     except kserrors.KickstartError, e:
         raise errors.KickstartError("Failed to parse kickstart file "
                                     "'%s' : %s" % (path, e))
@@ -153,8 +155,8 @@ class TimezoneConfig(KickstartConfig):
         try:
             shutil.copyfile(self.path("/usr/share/zoneinfo/%s" %(tz,)),
                             self.path("/etc/localtime"))
-        except OSError, (errno, msg):
-            log.error("Error copying timezone: %s" %(msg,))
+        except OSError, e:
+            log.error("Error copying timezone: %s" %(e.strerror,))
 
 
 class AuthConfig(KickstartConfig):