linux-kernel - Re: [PATCH v4 02/10] property: Add device_get_child_node_count

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: <1e0a9915-fe52-4569-9da0-b0761ba8fedb@gmail.com>
Date: Tue, 25 Feb 2025 12:56:56 +0200
From: Matti Vaittinen <mazziesaccount@...il.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
 Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Daniel Scally <djrscally@...il.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Danilo Krummrich <dakr@...nel.org>,
 Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
 Chen-Yu Tsai <wens@...e.org>, Jernej Skrabec <jernej.skrabec@...il.com>,
 Samuel Holland <samuel@...lland.org>,
 Hugo Villeneuve <hvilleneuve@...onoff.com>, Nuno Sa <nuno.sa@...log.com>,
 David Lechner <dlechner@...libre.com>,
 Javier Carrasco <javier.carrasco.cruz@...il.com>,
 Guillaume Stols <gstols@...libre.com>,
 Olivier Moysan <olivier.moysan@...s.st.com>,
 Dumitru Ceclan <mitrutzceclan@...il.com>,
 Trevor Gamblin <tgamblin@...libre.com>,
 Matteo Martelli <matteomartelli3@...il.com>,
 Alisa-Dariana Roman <alisadariana@...il.com>,
 Ramona Alexandra Nechita <ramona.nechita@...log.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
 linux-renesas-soc@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-sunxi@...ts.linux.dev
Subject: Re: [PATCH v4 02/10] property: Add
 device_get_child_node_count_named()

On 25/02/2025 12:21, Andy Shevchenko wrote:
> On Tue, Feb 25, 2025 at 11:40:16AM +0200, Heikki Krogerus wrote:
>>> +/**
>>> + * device_get_child_node_count_named - number of child nodes with given name
>>> + *
>>> + * Scan device's child nodes and find all the nodes with a specific name and
>>> + * return the number of found nodes. Potential '@...ber' -ending for scanned
>>> + * names is ignored. Eg,
>>> + * device_get_child_node_count(dev, "channel");
>>> + * would match all the nodes:
>>> + * channel { }, channel@0 {}, channel@...bba {}...
>>> + *
>>> + * @dev: Device to count the child nodes for
> 
> This has an inconsistent kernel doc structure in comparison to the rest in this
> file.

Hmm. I'll take a look at the differences for v5.

>>> + * Return: the number of child nodes with a matching name for a given device.
>>> + */
>>> +unsigned int device_get_child_node_count_named(const struct device *dev,
>>> +					       const char *name)
>>> +{
>>> +	struct fwnode_handle *child;
>>> +	unsigned int count = 0;
>>> +
>>> +	device_for_each_child_node(dev, child)
>>> +		if (fwnode_name_eq(child, "channel"))
>>
>> s/"channel"/name/ ?
>>
>>> +			count++;
>>> +
>>> +	return count;
>>> +}
>>> +EXPORT_SYMBOL_GPL(device_get_child_node_count_named);
>>
>> I did not check how many users are you proposing for this, but if
>> there's only one, then IMO this should not be a global function yet.
>> It just feels to special case to me. But let's see what the others
>> think.
> 
> The problem is that if somebody hides it, we might potentially see
> a duplication in the future. So I _slightly_ prefer to publish and
> then drop that after a few cycles if no users appear.
> 
> Also this misses the test cases.

I'll also take a look at the tests, but I have a bit of an attitude 
problem what comes to unit testing. Adding tests for the sake of having 
tests just hinders the development. It makes improving functions less 
appealing (as tests need to be changed as well) and adds bunch of 
inertia & maintenance cost. Sure, on complex functions having tests 
increases the confidence that changes work - but I don't see much value 
here.

Do we have tests for all the property.h functions?

Yours,
	-- Matti