linux-kernel - Re: [PATCH v1 1/1] test_printf: Refactor fwnode

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]

Date:   Thu, 29 Sep 2022 12:06:32 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Petr Mladek <pmladek@...e.com>, linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH v1 1/1] test_printf: Refactor fwnode_pointer() to make
 it more readable

On Wed, 24 Aug 2022 20:05:42 +0300
Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:

> Converting fwnode_pointer() to use better swnode API allows to
> make code more readable.
> 
> While at it, rename full_name to full_name_third to show exact
> relation in the hierarchy.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> ---
>  lib/test_printf.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/test_printf.c b/lib/test_printf.c
> index fe13de1bed5f..6f7f179dd8f4 100644
> --- a/lib/test_printf.c
> +++ b/lib/test_printf.c
> @@ -704,31 +704,29 @@ flags(void)
>  
>  static void __init fwnode_pointer(void)
>  {
> -	const struct software_node softnodes[] = {
> -		{ .name = "first", },
> -		{ .name = "second", .parent = &softnodes[0], },
> -		{ .name = "third", .parent = &softnodes[1], },
> -		{ NULL /* Guardian */ }
> -	};
> -	const char * const full_name = "first/second/third";
> +	const struct software_node first = { .name = "first" };
> +	const struct software_node second = { .name = "second", .parent = &first };
> +	const struct software_node third = { .name = "third", .parent = &second };

I personally do not find the above more readable, but honestly, I'm not
attached to this code at all.

> +	const struct software_node *group[] = { &first, &second, &third, NULL };

Could this just be:

	const struct software_node *group[] = {
		&softnodes[0], &softnodes[1], &softnodes[2], NULL };


>  	const char * const full_name_second = "first/second";
> +	const char * const full_name_third = "first/second/third";
>  	const char * const second_name = "second";
>  	const char * const third_name = "third";
>  	int rval;
>  
> -	rval = software_node_register_nodes(softnodes);
> +	rval = software_node_register_node_group(group);
>  	if (rval) {
>  		pr_warn("cannot register softnodes; rval %d\n", rval);
>  		return;
>  	}
>  
> -	test(full_name_second, "%pfw", software_node_fwnode(&softnodes[1]));
> -	test(full_name, "%pfw", software_node_fwnode(&softnodes[2]));
> -	test(full_name, "%pfwf", software_node_fwnode(&softnodes[2]));
> -	test(second_name, "%pfwP", software_node_fwnode(&softnodes[1]));
> -	test(third_name, "%pfwP", software_node_fwnode(&softnodes[2]));
> +	test(full_name_second, "%pfw", software_node_fwnode(&second));
> +	test(full_name_third, "%pfw", software_node_fwnode(&third));
> +	test(full_name_third, "%pfwf", software_node_fwnode(&third));
> +	test(second_name, "%pfwP", software_node_fwnode(&second));
> +	test(third_name, "%pfwP", software_node_fwnode(&third));

Then the above doesn't need to change.

But again, I'm not maintaining this code, so I'm not attached. Just adding
my $0.02 to this (as I'm triaging my inbox and found this email).

-- Steve


>  
> -	software_node_unregister_nodes(softnodes);
> +	software_node_unregister_node_group(group);
>  }
>  
>  static void __init fourcc_pointer(void)