From b7b89e5d2701a8c912518cc9e3c5cf6f3929c6f0 Mon Sep 17 00:00:00 2001 From: "Bryn M. Reeves" Date: Wed, 16 Apr 2014 17:40:34 +0100 Subject: Fix command parameter aliasing and exception handling The addition of 'runat' exposed a bug in the parameter passing for commands. We pack a tuple into a list in add_cmd_output() and then unpack it in collect_cmd_output(). These functions need to be careful in the use of positional vs. keyword arguments; passing keywords as though they were positional can lead to e.g. an integer timeout value appearing in the tuple member for 'suggest_filename' causing an exception when we attempt to pass the value to get_cmd_output_now(). Fix this by ensuring that all keyword args are passed explicitly through these functions and improve the debug logging by printing the tuple we are packing/unpacking. This also removes the exception handling block from the function collect_cmd_output() - this routine should allow exceptions to propagate up so that they can be handled by the --debug logic. The 'command not found' case is already logged in the generic get_command_output() wrapper. Signed-off-by: Bryn M. Reeves --- sos/plugins/__init__.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py index 20b62318..f7339638 100644 --- a/sos/plugins/__init__.py +++ b/sos/plugins/__init__.py @@ -440,7 +440,7 @@ class Plugin(object): self.log_debug("added copyspec '%s'" % copyspec) def get_command_output(self, prog, timeout=300, runat=None): - result = sos_get_command_output(prog, timeout, runat) + result = sos_get_command_output(prog, timeout=timeout, runat=runat) if result['status'] == 124: self.log_warn("command '%s' timed out after %ds" % (prog, timeout)) @@ -453,7 +453,7 @@ class Plugin(object): sosreport. """ # pylint: disable-msg = W0612 - return self.get_command_output(prog, timeout, runat) + return self.get_command_output(prog, timeout=timeout, runat=runat) def check_ext_prog(self, prog): """Execute a command independently of the output gathering part of @@ -467,15 +467,17 @@ class Plugin(object): if isinstance(cmds, six.string_types): raise TypeError("add_cmd_outputs called with string argument") for cmd in cmds: - self.add_cmd_output(cmd, timeout, runat) + self.add_cmd_output(cmd, timeout=timeout, runat=runat) def add_cmd_output(self, exe, suggest_filename=None, root_symlink=None, timeout=300, runat=None): """Run a program and collect the output""" cmd = (exe, suggest_filename, root_symlink, timeout, runat) + self.log_debug("packed command tuple: ('%s', '%s', '%s', %s, '%s')" + % cmd) self.collect_cmds.append(cmd) - self.log_debug("added cmd output '%s'" % exe) + self.log_info("added cmd output '%s'" % exe) def get_cmd_output_path(self, name=None, make=True): """Return a path into which this module should store collected @@ -529,7 +531,7 @@ class Plugin(object): report. """ # pylint: disable-msg = W0612 - result = self.get_command_output(exe, timeout, runat) + result = self.get_command_output(exe, timeout=timeout, runat=runat) if (result['status'] == 127): return None @@ -575,13 +577,12 @@ class Plugin(object): def collect_cmd_output(self): for progs in zip(self.collect_cmds): prog, suggest_filename, root_symlink, timeout, runat = progs[0] + self.log_debug("unpacked command tuple: " + + "('%s', '%s', '%s', %s, '%s')" % progs[0]) self.log_info("collecting output of '%s'" % prog) - try: - self.get_cmd_output_now(prog, suggest_filename, - root_symlink, timeout, runat) - except Exception as e: - self.log_debug("could not collect output of '%s': %s" - % (prog, e)) + self.get_cmd_output_now(prog, suggest_filename=suggest_filename, + root_symlink=root_symlink, + timeout=timeout, runat=runat) def collect_strings(self): for string, file_name in self.copy_strings: -- cgit