On Thursday, April 17, 2014 11:45:25 Ilia Mirkin wrote: > On Thu, Apr 17, 2014 at 11:36 AM, Dylan Baker <[email protected]> wrote: > > It seems perfectly safe to do something like this: > > > > def foo(defaults=[]): > > <stuff> > > > > But its not. Python will only initialize defaults once, and store it. > > Which means each time that foo is called with arguments defaults is > > changed. So when calling foo() more than once with arguments unexpected > > things can happen. This can create some very strange and hard to pin > > down bugs. > > I don't think that's quite right. The problem is that > > def foo(defaults=[]): > ... > > is the same as > > x = [] > def foo(defaults=x): > ... > > So if you modify "defaults" as part of your function, and it's not > passed in as an explicit argument, then future invocations of foo will > see the "modified" version if that argument wasn't supplied. It's a > disaster waiting to happen, but I'm pretty sure that the existing uses > were fine.
Right. I'll fix the commit message. > > > Signed-off-by: Dylan Baker <[email protected]> > > --- > > > > framework/core.py | 14 ++++++++------ > > tests/igt.py | 4 +++- > > 2 files changed, 11 insertions(+), 7 deletions(-) > > > > diff --git a/framework/core.py b/framework/core.py > > index 3979499..ac5be2b 100644 > > --- a/framework/core.py > > +++ b/framework/core.py > > > > @@ -344,8 +344,8 @@ class TestrunResult: > > class Environment: > > - def __init__(self, concurrent=True, execute=True, include_filter=[], > > - exclude_filter=[], valgrindúlse, dmesgúlse, > > verboseúlse): + def __init__(self, concurrent=True, execute=True, > > include_filter=None,> > > + exclude_filter=None, valgrindúlse, dmesgúlse, verboseúlse): > > self.concurrent = concurrent > > self.execute = execute > > self.filter = [] > > > > @@ -361,10 +361,12 @@ class Environment: > > This code uses re.compile to rebuild the lists and set > > self.filter > > """ > > > > - for each in include_filter: > > - self.filter.append(re.compile(each)) > > - for each in exclude_filter: > > - self.exclude_filter.append(re.compile(each)) > > + if include_filter is not None: > > + for each in include_filter: > > + self.filter.append(re.compile(each)) > > + if exclude_filter is not None: > > + for each in exclude_filter: > > + self.exclude_filter.append(re.compile(each)) > > A little trick I like to use is > > for each in include_filter or []: I definitely like that better. > ... > > But you don't have to do that, just an optional suggestion. > > Either way, > > Reviewed-by: Ilia Mirkin <[email protected]> > > > def __iter__(self): > > for key, values in self.__dict__.iteritems(): > > diff --git a/tests/igt.py b/tests/igt.py > > index 9c700a9..766f543 100644 > > --- a/tests/igt.py > > +++ b/tests/igt.py > > @@ -72,7 +72,9 @@ igtEnvironmentOk = checkEnvironment() > > > > profile = TestProfile() > > > > class IGTTest(Test): > > - def __init__(self, binary, arguments=[]): > > + def __init__(self, binary, arguments=None): > > + if arguments is None: > > + arguments = [] > > > > super(IGTTest, self).__init__( > > > > [path.join(igtTestRoot, binary)] + arguments) > > > > -- > > 1.9.2 > > > > _______________________________________________ > > Piglit mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
