On Mon, 23 Mar 2026 18:28:24 -0700 SeongJae Park <[email protected]> wrote:
> Forwarding Sashiko review for doing discussions via mails. > > # review url: > https://sashiko.dev/#/patchset/[email protected] > # start of sashiko.dev inline review > commit d9ca6c4a33fc4d613982730edda6f4b67ae30a67 > Author: SeongJae Park <[email protected]> > selftests/damon/_damon_sysfs: support pause file staging > > This commit adds support for the newly added pause file to the DAMON > test-purpose sysfs interface control Python module. > > > diff --git a/tools/testing/selftests/damon/_damon_sysfs.py > > b/tools/testing/selftests/damon/_damon_sysfs.py > > --- a/tools/testing/selftests/damon/_damon_sysfs.py > > +++ b/tools/testing/selftests/damon/_damon_sysfs.py > [ ... ] > > class DamonCtx: > > targets = None > > schemes = None > > kdamond = None > > + pause = None > > idx = None > > > > def __init__(self, ops='paddr', monitoring_attrs=DamonAttrs(), > > targets=[], > > - schemes=[]): > > + schemes=[], pause=False): > > This isn't a bug introduced by this patch, but since the function signature > is being modified here, is it a good time to address the mutable default > arguments? > > In Python, default arguments are evaluated exactly once at function > definition time. This means any instances of DamonCtx created without > explicitly providing targets, schemes, or monitoring_attrs will share > the same list structures and DamonAttrs instance in memory. > > For example, self.monitoring_attrs.context = self overwrites the context > attribute of the shared DamonAttrs object for all default contexts. > > Would it be better to change these defaults to None and instantiate them > within the method body to avoid unexpected state sharing across instances? I agree changing the defaults to None is a good idea. But that would better to be another patch, not this one. Thanks, SJ [...]
