Merge pull request #1003 from gpotter2/accuracy-log

More accurate ContextManagerSubprocess error messages

2 files changed, 20 insertions, 15 deletions

diff --git a/scapy/packet.py b/scapy/packet.py
index 50f0901f..9c17050d 100644
--- a/scapy/packet.py
+++ b/scapy/packet.py
@@ -497,7 +497,7 @@ class Packet(six.with_metaclass(Packet_metaclass, BasePacket)):
         if filename is None:
             fname = get_temp_file(autoext=".eps")
             canvas.writeEPSfile(fname)
-            with ContextManagerSubprocess("psdump()"):
+            with ContextManagerSubprocess("psdump()", conf.prog.psreader):
                 subprocess.Popen([conf.prog.psreader, fname])
         else:
             canvas.writeEPSfile(filename)
@@ -515,7 +515,7 @@ class Packet(six.with_metaclass(Packet_metaclass, BasePacket)):
         if filename is None:
             fname = get_temp_file(autoext=".pdf")
             canvas.writePDFfile(fname)
-            with ContextManagerSubprocess("pdfdump()"):
+            with ContextManagerSubprocess("pdfdump()", conf.prog.pdfreader):
                 subprocess.Popen([conf.prog.pdfreader, fname])
         else:
             canvas.writePDFfile(filename)
diff --git a/scapy/utils.py b/scapy/utils.py
index b6d481bb..8822e012 100644
--- a/scapy/utils.py
+++ b/scapy/utils.py
@@ -429,17 +429,21 @@ class ContextManagerSubprocess(object):
     >>>     subprocess.Popen(["unknown_command"])
 
     """
-    def __init__(self, name):
+    def __init__(self, name, prog):
         self.name = name
+        self.prog = prog
 
     def __enter__(self):
         pass
 
     def __exit__(self, exc_type, exc_value, traceback):
-        if exc_type == OSError:
-            msg = "%s: executing %r failed"
-            log_runtime.error(msg, self.name, conf.prog.wireshark, exc_info=1)
-            return True  # Suppress the exception
+        if isinstance(exc_value, (OSError, TypeError)):
+            msg = "%s: executing %r failed" % (self.name, self.prog) if self.prog else "Could not execute %s, is it installed ?" % self.name
+            if not conf.interactive:
+                raise OSError(msg)
+            else:
+                log_runtime.error(msg, exc_info=True)
+                return True  # Suppress the exception
 
 class ContextManagerCaptureOutput(object):
     """
@@ -500,7 +504,7 @@ def do_graph(graph,prog=None,format=None,target=None,type=None,string=None,optio
             target = get_temp_file(autoext="."+format)
             start_viewer = True
         else:
-            with ContextManagerSubprocess("do_graph()"):
+            with ContextManagerSubprocess("do_graph()", conf.prog.display):
                 target = subprocess.Popen([conf.prog.display],
                                           stdin=subprocess.PIPE).stdin
     if format is not None:
@@ -532,7 +536,7 @@ def do_graph(graph,prog=None,format=None,target=None,type=None,string=None,optio
             if conf.prog.display == conf.prog._default:
                 os.startfile(target.name)
             else:
-                with ContextManagerSubprocess("do_graph()"):
+                with ContextManagerSubprocess("do_graph()", conf.prog.display):
                     subprocess.Popen([conf.prog.display, target.name])
 
 _TEX_TR = {
@@ -1214,7 +1218,7 @@ def wireshark(pktlist):
     """Run wireshark on a list of packets"""
     f = get_temp_file()
     wrpcap(f, pktlist)
-    with ContextManagerSubprocess("wireshark()"):
+    with ContextManagerSubprocess("wireshark()", conf.prog.wireshark):
         subprocess.Popen([conf.prog.wireshark, "-r", f])
 
 @conf.commands.register
@@ -1275,15 +1279,16 @@ u'64'
         prog = [conf.prog.tcpdump]
     elif isinstance(prog, six.string_types):
         prog = [prog]
+    _prog_name = "windump()" if WINDOWS else "tcpdump()"
     if pktlist is None:
-        with ContextManagerSubprocess("tcpdump()"):
+        with ContextManagerSubprocess(_prog_name, prog[0]):
             proc = subprocess.Popen(
                 prog + (args if args is not None else []),
                 stdout=subprocess.PIPE if dump or getfd else None,
                 stderr=open(os.devnull) if quiet else None,
             )
     elif isinstance(pktlist, six.string_types):
-        with ContextManagerSubprocess("tcpdump()"):
+        with ContextManagerSubprocess(_prog_name, prog[0]):
             proc = subprocess.Popen(
                 prog + ["-r", pktlist] + (args if args is not None else []),
                 stdout=subprocess.PIPE if dump or getfd else None,
@@ -1299,7 +1304,7 @@ u'64'
             wrpcap(tmpfile, pktlist)
         else:
             tmpfile.close()
-        with ContextManagerSubprocess("tcpdump()"):
+        with ContextManagerSubprocess(_prog_name, prog[0]):
             proc = subprocess.Popen(
                 prog + ["-r", tmpfile.name] + (args if args is not None else []),
                 stdout=subprocess.PIPE if dump or getfd else None,
@@ -1307,7 +1312,7 @@ u'64'
             )
         conf.temp_files.append(tmpfile.name)
     else:
-        with ContextManagerSubprocess("tcpdump()"):
+        with ContextManagerSubprocess(_prog_name, prog[0]):
             proc = subprocess.Popen(
                 prog + ["-r", "-"] + (args if args is not None else []),
                 stdin=subprocess.PIPE,
@@ -1333,7 +1338,7 @@ def hexedit(x):
     x = str(x)
     f = get_temp_file()
     open(f,"wb").write(x)
-    with ContextManagerSubprocess("hexedit()"):
+    with ContextManagerSubprocess("hexedit()", conf.prog.hexedit):
         subprocess.call([conf.prog.hexedit, f])
     x = open(f).read()
     os.unlink(f)