[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formattin
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements |
Date: |
Tue, 19 Aug 2014 17:44:54 +0800 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Tue, 08/19 02:00, Maria Kustova wrote:
> Signed-off-by: Maria Kustova <address@hidden>
> ---
> tests/image-fuzzer/qcow2/fuzz.py | 15 ++++++------
> tests/image-fuzzer/runner.py | 51
> ++++++++++++++++++++--------------------
> 2 files changed, 34 insertions(+), 32 deletions(-)
>
> diff --git a/tests/image-fuzzer/qcow2/fuzz.py
> b/tests/image-fuzzer/qcow2/fuzz.py
> index 6e272c6..c652dc9 100644
> --- a/tests/image-fuzzer/qcow2/fuzz.py
> +++ b/tests/image-fuzzer/qcow2/fuzz.py
> @@ -123,7 +123,7 @@ def string_validator(current, strings):
> return validator(current, random.choice, strings)
>
>
> -def selector(current, constraints, validate=int_validator):
> +def selector(current, constraints, validate=None):
> """Select one value from all defined by constraints.
>
> Each constraint produces one random value satisfying to it. The function
> @@ -131,6 +131,9 @@ def selector(current, constraints,
> validate=int_validator):
> constraints overlaps).
> """
>
> + if validate is None:
> + validate = int_validator
> +
> def iter_validate(c):
> """Apply validate() only to constraints represented as lists.
>
> @@ -337,9 +340,8 @@ def l1_entry(current):
> constraints = UINT64_V
> # Reserved bits are ignored
> # Added a possibility when only flags are fuzzed
> - offset = 0x7fffffffffffffff & random.choice([selector(current,
> - constraints),
> - current])
> + offset = 0x7fffffffffffffff & \
> + random.choice([selector(current, constraints), current])
> is_cow = random.randint(0, 1)
> return offset + (is_cow << UINT64_M)
>
> @@ -349,9 +351,8 @@ def l2_entry(current):
> constraints = UINT64_V
> # Reserved bits are ignored
> # Add a possibility when only flags are fuzzed
> - offset = 0x3ffffffffffffffe & random.choice([selector(current,
> - constraints),
> - current])
> + offset = 0x3ffffffffffffffe & \
> + random.choice([selector(current, constraints), current])
> is_compressed = random.randint(0, 1)
> is_cow = random.randint(0, 1)
> is_zero = random.randint(0, 1)
> diff --git a/tests/image-fuzzer/runner.py b/tests/image-fuzzer/runner.py
> index fd97c40..b142577 100755
> --- a/tests/image-fuzzer/runner.py
> +++ b/tests/image-fuzzer/runner.py
> @@ -73,7 +73,7 @@ def run_app(fd, q_args):
> """Exception for signal.alarm events."""
> pass
>
> - def handler(*arg):
> + def handler(*args):
> """Notify that an alarm event occurred."""
> raise Alarm
>
> @@ -137,8 +137,8 @@ class TestEnv(object):
> self.init_path = os.getcwd()
> self.work_dir = work_dir
> self.current_dir = os.path.join(work_dir, 'test-' + test_id)
> - self.qemu_img = os.environ.get('QEMU_IMG', 'qemu-img')\
> - .strip().split(' ')
> + self.qemu_img = \
> + os.environ.get('QEMU_IMG', 'qemu-img').strip().split(' ')
> self.qemu_io = os.environ.get('QEMU_IO', 'qemu-io').strip().split('
> ')
> strings = ['%s%p%x%d', '.1024d', '%.2049d', '%p%p%p%p', '%x%x%x%x',
> '%d%d%d%d', '%s%s%s%s', '%99999999999s', '%08x', '%%20d',
> @@ -247,10 +247,8 @@ class TestEnv(object):
>
> os.chdir(self.current_dir)
> backing_file_name, backing_file_fmt = self._create_backing_file()
> - img_size = image_generator.create_image('test.img',
> - backing_file_name,
> - backing_file_fmt,
> - fuzz_config)
Actually this one is fine, but the new way is good too.
> + img_size = image_generator.create_image(
> + 'test.img', backing_file_name, backing_file_fmt, fuzz_config)
> for item in commands:
> shutil.copy('test.img', 'copy.img')
> # 'off' and 'len' are multiple of the sector size
> @@ -263,7 +261,7 @@ class TestEnv(object):
> elif item[0] == 'qemu-io':
> current_cmd = list(self.qemu_io)
> else:
> - multilog("Warning: test command '%s' is not defined.\n" \
> + multilog("Warning: test command '%s' is not defined.\n"
> % item[0], sys.stderr, self.log, self.parent_log)
> continue
> # Replace all placeholders with their real values
> @@ -279,29 +277,30 @@ class TestEnv(object):
> "Backing file: %s\n" \
> % (self.seed, " ".join(current_cmd),
> self.current_dir, backing_file_name)
> -
> temp_log = StringIO.StringIO()
> try:
> retcode = run_app(temp_log, current_cmd)
> except OSError, e:
> - multilog(test_summary + "Error: Start of '%s' failed. " \
> - "Reason: %s\n\n" % (os.path.basename(
> - current_cmd[0]), e[1]),
> + multilog(test_summary +
> + ("Error: Start of '%s' failed. Reason: %s\n\n"
> + % (os.path.basename(current_cmd[0]), e[1])),
I prefer the old one. I don't like the special case in python syntax: '(val)'
is just val, but '(val1, val2)' is a tuple.
> sys.stderr, self.log, self.parent_log)
> raise TestException
>
> if retcode < 0:
> self.log.write(temp_log.getvalue())
> - multilog(test_summary + "FAIL: Test terminated by signal " +
> - "%s\n\n" % str_signal(-retcode), sys.stderr,
> self.log,
> - self.parent_log)
> + multilog(test_summary +
> + ("FAIL: Test terminated by signal %s\n\n"
> + % str_signal(-retcode)),
> + sys.stderr, self.log, self.parent_log)
The same here.
> self.failed = True
> else:
> if self.log_all:
> self.log.write(temp_log.getvalue())
> - multilog(test_summary + "PASS: Application exited with" +
> - " the code '%d'\n\n" % retcode, sys.stdout,
> - self.log, self.parent_log)
> + multilog(test_summary +
> + ("PASS: Application exited with the code
> '%d'\n\n"
> + % retcode),
> + sys.stdout, self.log, self.parent_log)
And here.
Fam
> temp_log.close()
> os.remove('copy.img')
>
> @@ -321,8 +320,9 @@ if __name__ == '__main__':
>
> Set up test environment in TEST_DIR and run a test in it. A module
> for
> test image generation should be specified via IMG_GENERATOR.
> +
> Example:
> - runner.py -c '[["qemu-img", "info", "$test_img"]]' /tmp/test ../qcow2
> + runner.py -c '[["qemu-img", "info", "$test_img"]]' /tmp/test
> ../qcow2
>
> Optional arguments:
> -h, --help display this help and exit
> @@ -340,20 +340,22 @@ if __name__ == '__main__':
>
> '--command' accepts a JSON array of commands. Each command presents
> an application under test with all its paramaters as a list of
> strings,
> - e.g.
> - ["qemu-io", "$test_img", "-c", "write $off $len"]
> + e.g. ["qemu-io", "$test_img", "-c", "write $off $len"].
>
> Supported application aliases: 'qemu-img' and 'qemu-io'.
> +
> Supported argument aliases: $test_img for the fuzzed image, $off
> for an offset, $len for length.
>
> Values for $off and $len will be generated based on the virtual disk
> - size of the fuzzed image
> + size of the fuzzed image.
> +
> Paths to 'qemu-img' and 'qemu-io' are retrevied from 'QEMU_IMG' and
> - 'QEMU_IO' environment variables
> + 'QEMU_IO' environment variables.
>
> '--config' accepts a JSON array of fields to be fuzzed, e.g.
> - '[["header"], ["header", "version"]]'
> + '[["header"], ["header", "version"]]'.
> +
> Each of the list elements can consist of a complex image element only
> as ["header"] or ["feature_name_table"] or an exact field as
> ["header", "version"]. In the first case random portion of the
> element
> @@ -403,7 +405,6 @@ if __name__ == '__main__':
> seed = None
> config = None
> duration = None
> -
> for opt, arg in opts:
> if opt in ('-h', '--help'):
> usage()
> --
> 1.9.3
>