linux-kernel - Re: [RFC PATCH 10/77] tests: Add basic metadata tests

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: <aW3B0GHnMgwF73oK@zatzit>
Date: Mon, 19 Jan 2026 16:32:00 +1100
From: David Gibson <david@...son.dropbear.id.au>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Ayush Singh <ayush@...gleboard.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	devicetree-compiler@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree-spec@...r.kernel.org,
	Hui Pu <hui.pu@...ealthcare.com>,
	Ian Ray <ian.ray@...ealthcare.com>,
	Luca Ceresoli <luca.ceresoli@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [RFC PATCH 10/77] tests: Add basic metadata tests

On Fri, Jan 16, 2026 at 02:36:00PM +0100, Herve Codina wrote:
> On Thu, 15 Jan 2026 11:50:56 +1100
> David Gibson <david@...son.dropbear.id.au> wrote:
> 
> > On Mon, Jan 12, 2026 at 03:19:00PM +0100, Herve Codina wrote:
> > > This first test is related to local phandle references (FDT_REF_LOCAL
> > > dtb tag).
> > > 
> > > The test pattern used is
> > >   - Generate a dts (xxx.dts.dts) from an input dts
> > >   - Check this generated dts against expected contents
> > >   - Generate a dtb (xxx.dtb) from the same input dts
> > >   - Check this generated dtb against expected contents
> > >   - Generate a dts (xxx.dtb.dts) from the generated dtb
> > >   - Check this generated dts against expected contents
> > >   - Generate a dtb (xxx.dtb.dts.dtb) from the generated dts
> > >   - Check this generated dtb, expect the same contents as for xxx.dtb
> > > 
> > > Even if only one meta-data feature is currently tested in this tests
> > > introduction, use a loop in order to ease future addition consisting in
> > > adding new input dts as soon as new meta-data feature are supported.  
> > 
> > As a rule of tumb, I prefer tests to be introduced in the same patch
> > that introduces the feature tested.  Usually, I don't care that much,
> > but in a giant series like this, it would really help review (the
> > tests help to document the feature being added without switching
> > between patches).
> 
> Hum ok but it is worth noting that a feature needs several patches to be
> fully supported.

Right, it's a general guideline, not a hard and fast rule.

In most cases I'd suggest putting the tests in with the patch that
adds the piece they need to work.  That does mean that if some of the
simpler tests can operate with only some of the patches, moving those
tests earlier.

Again, only a guideline, but it helps review because the tests act as
documentation for the functionality the patch is adding.

> In order to ease the review (maybe I was wrong), I chose to have distinct
> patches for modification related to new dts keywords and for modification
> related to new dtb tag and tried to have patches as small as
> possible.

Small is usually good.  However, if it's so small that the patch
doesn't express a complete idea so you have to keep referencing
surrounding patches to make sense of it, then it's no longer good.
Fwiw, think these patches are mostly well divided, but as above,
folding tests in with code changes is usually better because the tests
help show what the new code is expected to do.

[I will note that having the tests as a separate patch, usuallly
*before* the code changes is very useful during development.  But git
makes it pretty easy to re-order and fold things together when you're
ready to post]

> The last patch of a new feature was a patch adding test.
> 
> I am open to remove patches that just add tests. In that case tests will be
> added in the last patch related to a new feature. You still need to switch
> between patches in that case.

Yes, but not quite as much.  And even less if those tests which can be
moved earlier are moved earlier.

> Further more, during iteration, the last patch of a new feature could be
> modified just because the test part is present in this patch even if other
> part of the patch itself is not impacted.
> 
> I mean:
>   - patch 1: related to dts keyword
>   - patch 2: related to dtb + test
> 
> Update patch 1 will imply an update to patch 2.
> I am not sure that this will ease the review.

Point taken.  For tests that intrinsically require things from
multiple earlier patches, having them separate probably makes sense.
Often at least some of the tests can be more closely related to a
single patch though.  When possible, that's generally preferable.

> To avoid that, I can merge all patches related to feature into only one
> patch. This patch can be quite huge and I am not sure that it will be easy
> to review it.

No, definitely don't do that.

> Once again, I am open to merging patches. Let me know what do you prefer
> in terms of patch granularity.
> 
> Best regards,
> Hervé
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)