From 26692e749c87a75550bb5d0a7c18857a5202b85a Mon Sep 17 00:00:00 2001 From: Thibault Saunier Date: Thu, 22 Dec 2016 10:07:58 -0300 Subject: [PATCH] validate:launcher: Stop running test subprocesses in a shell And instead properly use a list of argument for the subprocesses. --- validate/launcher/apps/gstvalidate.py | 10 +++---- validate/launcher/baseclasses.py | 56 +++++++++++++++++------------------ 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/validate/launcher/apps/gstvalidate.py b/validate/launcher/apps/gstvalidate.py index 711ee78..39c400b 100644 --- a/validate/launcher/apps/gstvalidate.py +++ b/validate/launcher/apps/gstvalidate.py @@ -20,6 +20,7 @@ import argparse import os import time import urllib.parse +import shlex import subprocess import configparser from launcher.loggable import Loggable @@ -411,10 +412,10 @@ class GstValidateLaunchTest(GstValidateTest): def build_arguments(self): GstValidateTest.build_arguments(self) - self.add_arguments(self.pipeline_desc) + self.add_arguments(*shlex.split(self.pipeline_desc)) if self.media_descriptor is not None and self.media_descriptor.get_path(): self.add_arguments( - "--set-media-info", '"' + self.media_descriptor.get_path() + '"') + "--set-media-info", self.media_descriptor.get_path()) class GstValidateMediaCheckTest(GstValidateTest): @@ -437,8 +438,7 @@ class GstValidateMediaCheckTest(GstValidateTest): def build_arguments(self): Test.build_arguments(self) - self.add_arguments(self._uri, "--expected-results", - '"' + self._media_info_path + '"') + self.add_arguments(self._uri, "--expected-results", self._media_info_path) class GstValidateTranscodingTest(GstValidateTest, GstValidateEncodingTestInterface): @@ -500,7 +500,7 @@ class GstValidateTranscodingTest(GstValidateTest, GstValidateEncodingTestInterfa def build_arguments(self): GstValidateTest.build_arguments(self) self.set_rendering_info() - self.add_arguments(self.uri, '"' + self.dest_file + '"') + self.add_arguments(self.uri, self.dest_file) def get_current_value(self): if self.scenario: diff --git a/validate/launcher/baseclasses.py b/validate/launcher/baseclasses.py index 20053b6..2b2ee69 100644 --- a/validate/launcher/baseclasses.py +++ b/validate/launcher/baseclasses.py @@ -75,7 +75,7 @@ class Test(Loggable): self.classname = classname self.options = options self.application = application_name - self.command = "" + self.command = [] self.reporter = reporter self.process = None self.proc_env = None @@ -115,7 +115,7 @@ class Test(Loggable): if self.result in [Result.FAILED, Result.TIMEOUT]: string += " '%s'\n" \ " You can reproduce with: %s %s\n" \ - % (self.message, self._env_variable, self.command) + % (self.message, self._env_variable, ' '.join(self.command)) if not self.options.redirect_logs: string += " You can find logs in:\n" \ @@ -146,7 +146,7 @@ class Test(Loggable): res += " " value = self.proc_env.get(var, None) if value: - res += "%s=%s" % (var, value) + res += "%s='%s'" % (var, value) return res @@ -195,8 +195,7 @@ class Test(Loggable): return self.classname.split('.')[-1] def add_arguments(self, *args): - for arg in args: - self.command += " " + arg + self.command += args def build_arguments(self): self.add_env_variable("LD_PRELOAD") @@ -345,10 +344,9 @@ class Test(Loggable): res = self.process.poll() def thread_wrapper(self): - self.process = subprocess.Popen("exec " + self.command, + self.process = subprocess.Popen(self.command, stderr=self.out, stdout=self.out, - shell=True, env=self.proc_env) self.process.wait() if self.result is not Result.TIMEOUT: @@ -361,31 +359,32 @@ class Test(Loggable): if self.hard_timeout is not None: self.hard_timeout *= GDB_TIMEOUT_FACTOR self.timeout *= GDB_TIMEOUT_FACTOR - self.command = "gdb -ex run -ex quit --args %s" % self.command + self.command = ["gdb", "-ex", "run", "-ex", "quit", + "--args"] + self.command def use_valgrind(self): vglogsfile = self.logfile + '.valgrind' self.extra_logfiles.append(vglogsfile) - vg_args = [ - ('trace-children', 'yes'), - ('tool', 'memcheck'), - ('leak-check', 'full'), - ('leak-resolution', 'high'), - # TODO: errors-for-leak-kinds should be set to all instead of definite - # and all false positives should be added to suppression files. - ('errors-for-leak-kinds', 'definite'), - ('num-callers', '20'), - ('log-file', '"' + vglogsfile + '"'), - ('error-exitcode', str(VALGRIND_ERROR_CODE)), - ('gen-suppressions', 'all'), - ] + vg_args = [] + + for o, v in [('trace-children', 'yes'), + ('tool', 'memcheck'), + ('leak-check', 'full'), + ('leak-resolution', 'high'), + # TODO: errors-for-leak-kinds should be set to all instead of definite + # and all false positives should be added to suppression files. + ('errors-for-leak-kinds', 'definite'), + ('num-callers', '20'), + ('log-file', vglogsfile), + ('error-exitcode', str(VALGRIND_ERROR_CODE)), + ('gen-suppressions', 'all')]: + vg_args.append("--%s=%s" % (o, v)) for supp in self.get_valgrind_suppressions(): - vg_args.append(('suppressions', supp)) + vg_args.append("--suppressions=%s" % supp) - self.command = "valgrind %s %s" % (' '.join(['--%s=%s' % (x[0], x[1]) for x in vg_args]), - self.command) + self.command = ["valgrind"] + vg_args + self.command # Tune GLib's memory allocator to be more valgrind friendly self.proc_env['G_DEBUG'] = 'gc-friendly' @@ -412,7 +411,7 @@ class Test(Loggable): self.open_logfile() self.queue = queue - self.command = "%s " % (self.application) + self.command = [self.application] self._starting_time = time.time() self.build_arguments() self.proc_env = self.get_subproc_env() @@ -430,7 +429,7 @@ class Test(Loggable): message = "Launching: %s%s\n" \ " Command: '%s %s'\n" % (Colors.ENDC, self.classname, - self._env_variable, self.command) + self._env_variable, ' '.join(self.command)) if not self.options.redirect_logs: message += " Logs:\n" \ " - %s" % (self.logfile) @@ -441,7 +440,7 @@ class Test(Loggable): "Test name: %s\n" "Command: '%s'\n" "=================\n\n" - % (self.classname, self.command)) + % (self.classname, ' '.join(self.command))) self.out.flush() printc(message, Colors.OKBLUE) @@ -888,7 +887,7 @@ class GstValidateEncodingTestInterface(object): def _get_profile_full(self, muxer, venc, aenc, video_restriction=None, audio_restriction=None, audio_presence=0, video_presence=0): - ret = "\"" + ret = "" if muxer: ret += muxer ret += ":" @@ -906,7 +905,6 @@ class GstValidateEncodingTestInterface(object): if audio_presence: ret = ret + '|' + str(audio_presence) - ret += "\"" return ret.replace("::", ":") def get_profile(self, video_restriction=None, audio_restriction=None): -- 2.7.4