<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" "http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" content="1" /><style type="text/css"> p, li { white-space: pre-wrap; } </style></head><body style=" font-family:'DejaVu Sans Mono'; font-size:9pt; font-weight:400; font-style:normal;"> On Thursday, April 17, 2014 11:45:25 Ilia Mirkin wrote: > On Thu, Apr 17, 2014 at 11:36 AM, Dylan Baker <baker.dylan.c@gmail.com> 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 <baker.dylan.c@gmail.com> > > --- > > > > 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=False, dmesg=False, > > verbose=False): + def __init__(self, concurrent=True, execute=True, > > include_filter=None,> > > + exclude_filter=None, valgrind=False, dmesg=False, verbose=False): > > 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 <imirkin@alum.mit.edu> > > > 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 > > Piglit@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/piglit </body></html>