netdev - Re: [PATCH net-next 7/7] testing: net-drv: add a driver test for stats reporting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ttkigynz.fsf@nvidia.com>
Date: Wed, 3 Apr 2024 18:52:50 +0200
From: Petr Machata <petrm@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: Petr Machata <petrm@...dia.com>, <davem@...emloft.net>,
	<netdev@...r.kernel.org>, <edumazet@...gle.com>, <pabeni@...hat.com>,
	<shuah@...nel.org>, <sdf@...gle.com>, <donald.hunter@...il.com>,
	<linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH net-next 7/7] testing: net-drv: add a driver test for
 stats reporting


Jakub Kicinski <kuba@...nel.org> writes:

> On Wed, 3 Apr 2024 10:58:19 +0200 Petr Machata wrote:
>> Also, it's not clear what "del thing" should do in that context, because
>> if cfg also keeps a reference, __del__ won't get called. There could be
>> a direct method, like thing.exit() or whatever, but then you need
>> bookkeeping so as not to clean up the second time through cfg. It's the
>> less straightforward way of going about it IMHO.
>
> I see, having read up on what del actually does - "del thing" would
> indeed not work here.
>
>> I know that I must sound like a broken record at this point, but look:
>> 
>>     with build("ip link set dev %s master %s" % (swp1, h1),
>>                "ip link set dev %s nomaster" % swp1) as thing:
>>         ... some code which may rise ...
>>     ... more code, interface detached, `thing' gone ...
>> 
>> It's just as concise, makes it very clear where the device is part of
>> the bridge and where not anymore, and does away with the intricacies of
>> lifetime management.
>
> My experience [1] is that with "with" we often end up writing tests
> like this:
>
> 	def test():
> 		with a() as bunch,
> 		     of() as things:
> 			... entire body of the test indented ...
>
> [1] https://github.com/kuba-moo/linux/blob/psp/tools/net/ynl/psp.py

Yeah, that does end up happening. I think there are a couple places
where you could have folded several withs in one, but it is going to be
indented, yeah.

But you end up indenting for try: finally: to make the del work reliably
anyway, so it's kinda lose/lose in that regard.

> Nothing wrong with that. I guess the question in my mind is whether
> we're aiming for making the tests "pythonic" (in which case "with"
> definitely wins), or more of a "bash with classes" style trying to
> avoid any constructs people may have to google. I'm on the fence on
> that one, as the del example proves my python expertise is not high.
> OTOH people who prefer bash will continue to write bash tests,
> so maybe we don't have to worry about non-experts too much. Dunno.

What I'm saying is, bash is currently a bit of a mess when it comes to
cleanups. It's hard to get right, annoying to review, and sometimes
individual cases add state that they don't unwind in cleanup() but only
later in the function, so when you C-c half-way through such case, stuff
stays behind.

Python has tools to just magic all this away.