[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formattin
From: |
M.Kustova |
Subject: |
Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements |
Date: |
Tue, 19 Aug 2014 13:55:16 +0400 |
On Tue, Aug 19, 2014 at 1:44 PM, Fam Zheng <address@hidden> wrote:
> 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.
This 'tuple' format provides that '%' substitution will be applied
only to the string in the parentheses,
instead of an entire string (in this case including test summary).
The same rationale for cases below.
>
>> 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
>>