Re: [Qemu-devel] [PATCH] image-fuzzer: Trivial readability and formatting improvements

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

On Tue, Aug 19, 2014 at 01:50:27PM +0400, M.Kustova wrote:
> On Tue, Aug 19, 2014 at 1:38 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Tue, Aug 19, 2014 at 02:00:24AM +0400, Maria Kustova wrote:
> >> 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.
> >>
> >
> > I don't understand the benefit of this change.  The default initializer
> > syntax did what was intended, now it has been made more complicated with
> > a None value that gets converted to int_validator later.
> 
> It's a convention rule that allows keep the function signature
> unchanged in case of modified function internals.
> In other words, wrapper functions should not change in response of the
> internal function change.

I don't think that is necessary.  This is not a stable API that cannot
change.

Also, what exactly creates an API commitment here?  Hiding int_validator
doesn't hide anything.

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]