[PATCH 00/12] samples: configfs: refactor the configfs sample code

[PATCH 00/12] samples: configfs: refactor the configfs sample code

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Over at the GPIO sub-system we have a testing module (gpio-mockup) which
uses module params to instantiate simulated GPIO chips and debugfs to
control them. We want to switch to a more stable interface using configfs
to instantiate chips and sysfs to control them.

For that we need the feature that's well defined in the docs but currently
unimplemented - committable items. I started working on this but it won't
be ready for this release cycle.

In the meantime I also refactored the configfs samples a bit as I'll be
adding sample code for committable items. I thought that it won't hurt
to send the refactoring patches now for v5.10 so here it is. No logical
changes really, mostly just code quality and removing usage of deprecated
functions.

Bartosz Golaszewski (12):
  MAINTAINERS: add the sample directory to the configfs entry
  samples: configfs: order includes alphabetically
  samples: configfs: remove redundant newlines
  samples: configfs: drop unnecessary ternary operators
  samples: configfs: improve alignment of broken lines
  samples: configfs: fix alignment in item struct
  samples: configfs: replace simple_strtoul() with kstrtoint()
  samples: configfs: don't reinitialize variables which are already
    zeroed
  samples: configfs: prefer sizeof(*var) to sizeof(struct var_type)
  samples: configfs: consolidate local variables of the same type
  samples: configfs: don't use spaces before tabs
  samples: configfs: prefer pr_err() over bare printk(KERN_ERR

 MAINTAINERS                        |  1 +
 samples/configfs/configfs_sample.c | 78 +++++++++++-------------------
 2 files changed, 29 insertions(+), 50 deletions(-)

-- 
2.17.1

[PATCH 01/12] MAINTAINERS: add the sample directory to the configfs entry

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Code samples for configfs don't have an explicit maintainer. Add the
samples directory to the existing configfs entry in MAINTAINERS.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d746519253c3..6bbe4cb67331 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4407,6 +4407,7 @@ S:	Supported
 T:	git git://git.infradead.org/users/hch/configfs.git
 F:	fs/configfs/
 F:	include/linux/configfs.h
+F:	samples/configfs/
 
 CONNECTOR
 M:	Evgeniy Polyakov <zbr@ioremap.net>
-- 
2.17.1

[PATCH 02/12] samples: configfs: order includes alphabetically

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The preferred coding style is to order all includes alphabetically for
improved readability. There's no need for the configfs header to come
last.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index e2398d94e8da..aca994e6ab87 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -12,11 +12,11 @@
  * configfs Copyright (C) 2005 Oracle.  All rights reserved.
  */
 
+#include <linux/configfs.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 
-#include <linux/configfs.h>
 
 
 
-- 
2.17.1

[PATCH 03/12] samples: configfs: remove redundant newlines

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

There's no need for suplemental newlines in the source file - especially
since the examples are well divided with comments already.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index aca994e6ab87..d3370180bcce 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -17,9 +17,6 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 
-
-
-
 /*
  * 01-childless
  *
@@ -117,7 +114,6 @@ static struct childless childless_subsys = {
 	},
 };
 
-
 /* ----------------------------------------------------------------- */
 
 /*
@@ -185,7 +181,6 @@ static const struct config_item_type simple_child_type = {
 	.ct_owner	= THIS_MODULE,
 };
 
-
 struct simple_children {
 	struct config_group group;
 };
@@ -263,7 +258,6 @@ static struct configfs_subsystem simple_children_subsys = {
 	},
 };
 
-
 /* ----------------------------------------------------------------- */
 
 /*
-- 
2.17.1

[PATCH 04/12] samples: configfs: drop unnecessary ternary operators

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Checking pointers for NULL value before passing them to container_of()
is pointless because even if we return NULL from the ternary operator,
none of the users checks the returned value but they instead dereference
it unconditionally. AFAICT this cannot really happen either. Simplify
the code by removing the ternary operators from to_childless() et al.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index d3370180bcce..e339404dce1c 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -37,8 +37,8 @@ struct childless {
 
 static inline struct childless *to_childless(struct config_item *item)
 {
-	return item ? container_of(to_configfs_subsystem(to_config_group(item)),
-			struct childless, subsys) : NULL;
+	return container_of(to_configfs_subsystem(to_config_group(item)),
+			    struct childless, subsys);
 }
 
 static ssize_t childless_showme_show(struct config_item *item, char *page)
@@ -132,7 +132,7 @@ struct simple_child {
 
 static inline struct simple_child *to_simple_child(struct config_item *item)
 {
-	return item ? container_of(item, struct simple_child, item) : NULL;
+	return container_of(item, struct simple_child, item);
 }
 
 static ssize_t simple_child_storeme_show(struct config_item *item, char *page)
@@ -187,8 +187,8 @@ struct simple_children {
 
 static inline struct simple_children *to_simple_children(struct config_item *item)
 {
-	return item ? container_of(to_config_group(item),
-			struct simple_children, group) : NULL;
+	return container_of(to_config_group(item),
+			    struct simple_children, group);
 }
 
 static struct config_item *simple_children_make_item(struct config_group *group,
-- 
2.17.1

[PATCH 05/12] samples: configfs: improve alignment of broken lines

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Improve the coding style by correctly aligning broken lines where
possible.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index e339404dce1c..924b57258af0 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -58,7 +58,7 @@ static ssize_t childless_storeme_show(struct config_item *item, char *page)
 }
 
 static ssize_t childless_storeme_store(struct config_item *item,
-		const char *page, size_t count)
+				       const char *page, size_t count)
 {
 	struct childless *childless = to_childless(item);
 	unsigned long tmp;
@@ -141,7 +141,7 @@ static ssize_t simple_child_storeme_show(struct config_item *item, char *page)
 }
 
 static ssize_t simple_child_storeme_store(struct config_item *item,
-		const char *page, size_t count)
+					  const char *page, size_t count)
 {
 	struct simple_child *simple_child = to_simple_child(item);
 	unsigned long tmp;
@@ -192,7 +192,7 @@ static inline struct simple_children *to_simple_children(struct config_item *ite
 }
 
 static struct config_item *simple_children_make_item(struct config_group *group,
-		const char *name)
+						     const char *name)
 {
 	struct simple_child *simple_child;
 
@@ -209,7 +209,7 @@ static struct config_item *simple_children_make_item(struct config_group *group,
 }
 
 static ssize_t simple_children_description_show(struct config_item *item,
-		char *page)
+						char *page)
 {
 	return sprintf(page,
 "[02-simple-children]\n"
@@ -270,8 +270,8 @@ static struct configfs_subsystem simple_children_subsys = {
  * children of its own.
  */
 
-static struct config_group *group_children_make_group(
-		struct config_group *group, const char *name)
+static struct config_group *
+group_children_make_group(struct config_group *group, const char *name)
 {
 	struct simple_children *simple_children;
 
-- 
2.17.1

[PATCH 06/12] samples: configfs: fix alignment in item struct

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Aling the assignment of a static structure's field to be consistent with
all other instances.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index 924b57258af0..3e6f0e0b0f72 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -172,7 +172,7 @@ static void simple_child_release(struct config_item *item)
 }
 
 static struct configfs_item_operations simple_child_item_ops = {
-	.release		= simple_child_release,
+	.release	= simple_child_release,
 };
 
 static const struct config_item_type simple_child_type = {
-- 
2.17.1

[PATCH 07/12] samples: configfs: replace simple_strtoul() with kstrtoint()

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

simple_strtoul() is deprecated. Use kstrtoint().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index 3e6f0e0b0f72..d89a1ffea620 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -14,6 +14,7 @@
 
 #include <linux/configfs.h>
 #include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/slab.h>
 
@@ -61,17 +62,11 @@ static ssize_t childless_storeme_store(struct config_item *item,
 				       const char *page, size_t count)
 {
 	struct childless *childless = to_childless(item);
-	unsigned long tmp;
-	char *p = (char *) page;
-
-	tmp = simple_strtoul(p, &p, 10);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
-
-	if (tmp > INT_MAX)
-		return -ERANGE;
+	int ret;
 
-	childless->storeme = tmp;
+	ret = kstrtoint(page, 10, &childless->storeme);
+	if (ret)
+		return ret;
 
 	return count;
 }
@@ -144,17 +139,11 @@ static ssize_t simple_child_storeme_store(struct config_item *item,
 					  const char *page, size_t count)
 {
 	struct simple_child *simple_child = to_simple_child(item);
-	unsigned long tmp;
-	char *p = (char *) page;
-
-	tmp = simple_strtoul(p, &p, 10);
-	if (!p || (*p && (*p != '\n')))
-		return -EINVAL;
-
-	if (tmp > INT_MAX)
-		return -ERANGE;
+	int ret;
 
-	simple_child->storeme = tmp;
+	ret = kstrtoint(page, 10, &simple_child->storeme);
+	if (ret)
+		return ret;
 
 	return count;
 }
-- 
2.17.1

[PATCH 08/12] samples: configfs: don't reinitialize variables which are already zeroed

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The structure containing the storeme field is allocated using kzalloc().
There's no need to set it to 0 again.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index d89a1ffea620..20bf3b3e0f0f 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -192,8 +192,6 @@ static struct config_item *simple_children_make_item(struct config_group *group,
 	config_item_init_type_name(&simple_child->item, name,
 				   &simple_child_type);
 
-	simple_child->storeme = 0;
-
 	return &simple_child->item;
 }
 
-- 
2.17.1

[PATCH 09/12] samples: configfs: prefer sizeof(*var) to sizeof(struct var_type)

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

It's better to use the size of the actual variable than its type when
allocating memory. This also has the benefit of avoiding a line break
here.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index 20bf3b3e0f0f..220aee7075b6 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -185,7 +185,7 @@ static struct config_item *simple_children_make_item(struct config_group *group,
 {
 	struct simple_child *simple_child;
 
-	simple_child = kzalloc(sizeof(struct simple_child), GFP_KERNEL);
+	simple_child = kzalloc(sizeof(*simple_child), GFP_KERNEL);
 	if (!simple_child)
 		return ERR_PTR(-ENOMEM);
 
@@ -262,8 +262,7 @@ group_children_make_group(struct config_group *group, const char *name)
 {
 	struct simple_children *simple_children;
 
-	simple_children = kzalloc(sizeof(struct simple_children),
-				  GFP_KERNEL);
+	simple_children = kzalloc(sizeof(*simple_children), GFP_KERNEL);
 	if (!simple_children)
 		return ERR_PTR(-ENOMEM);
 
-- 
2.17.1

[PATCH 10/12] samples: configfs: consolidate local variables of the same type

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Move local variables of the same type into a single line for better
readability.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index 220aee7075b6..49c87ca58116 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -330,9 +330,8 @@ static struct configfs_subsystem *example_subsys[] = {
 
 static int __init configfs_example_init(void)
 {
-	int ret;
-	int i;
 	struct configfs_subsystem *subsys;
+	int ret, i;
 
 	for (i = 0; example_subsys[i]; i++) {
 		subsys = example_subsys[i];
-- 
2.17.1

[PATCH 11/12] samples: configfs: don't use spaces before tabs

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

The copyright notice alarms checkpatch.pl of usin spaces before tabs.
Fix this.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index 49c87ca58116..da4e0f4ec20a 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -7,7 +7,7 @@
  *      macros defined by configfs.h
  *
  * Based on sysfs:
- * 	sysfs is Copyright (C) 2001, 2002, 2003 Patrick Mochel
+ *      sysfs is Copyright (C) 2001, 2002, 2003 Patrick Mochel
  *
  * configfs Copyright (C) 2005 Oracle.  All rights reserved.
  */
-- 
2.17.1

[PATCH 12/12] samples: configfs: prefer pr_err() over bare printk(KERN_ERR

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

pr_*() printing helpers are preferred over using bare printk().

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 samples/configfs/configfs_sample.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/samples/configfs/configfs_sample.c b/samples/configfs/configfs_sample.c
index da4e0f4ec20a..2f3b26d1d45e 100644
--- a/samples/configfs/configfs_sample.c
+++ b/samples/configfs/configfs_sample.c
@@ -340,9 +340,8 @@ static int __init configfs_example_init(void)
 		mutex_init(&subsys->su_mutex);
 		ret = configfs_register_subsystem(subsys);
 		if (ret) {
-			printk(KERN_ERR "Error %d while registering subsystem %s\n",
-			       ret,
-			       subsys->su_group.cg_item.ci_namebuf);
+			pr_err("Error %d while registering subsystem %s\n",
+			       ret, subsys->su_group.cg_item.ci_namebuf);
 			goto out_unregister;
 		}
 	}
-- 
2.17.1

Re: [PATCH 00/12] samples: configfs: refactor the configfs sample code

[Bartosz Golaszewski]

On Thu, Sep 24, 2020 at 2:45 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Over at the GPIO sub-system we have a testing module (gpio-mockup) which
> uses module params to instantiate simulated GPIO chips and debugfs to
> control them. We want to switch to a more stable interface using configfs
> to instantiate chips and sysfs to control them.
>
> For that we need the feature that's well defined in the docs but currently
> unimplemented - committable items. I started working on this but it won't
> be ready for this release cycle.
>
> In the meantime I also refactored the configfs samples a bit as I'll be
> adding sample code for committable items. I thought that it won't hurt
> to send the refactoring patches now for v5.10 so here it is. No logical
> changes really, mostly just code quality and removing usage of deprecated
> functions.
>
> Bartosz Golaszewski (12):
>   MAINTAINERS: add the sample directory to the configfs entry
>   samples: configfs: order includes alphabetically
>   samples: configfs: remove redundant newlines
>   samples: configfs: drop unnecessary ternary operators
>   samples: configfs: improve alignment of broken lines
>   samples: configfs: fix alignment in item struct
>   samples: configfs: replace simple_strtoul() with kstrtoint()
>   samples: configfs: don't reinitialize variables which are already
>     zeroed
>   samples: configfs: prefer sizeof(*var) to sizeof(struct var_type)
>   samples: configfs: consolidate local variables of the same type
>   samples: configfs: don't use spaces before tabs
>   samples: configfs: prefer pr_err() over bare printk(KERN_ERR
>
>  MAINTAINERS                        |  1 +
>  samples/configfs/configfs_sample.c | 78 +++++++++++-------------------
>  2 files changed, 29 insertions(+), 50 deletions(-)
>
> --
> 2.17.1
>

Hi Joel, Christoph,

Any comments on this? Can this still go into v5.10?

Bartosz

Re: [PATCH 02/12] samples: configfs: order includes alphabetically

[Christoph Hellwig]

On Thu, Sep 24, 2020 at 02:45:16PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> The preferred coding style is to order all includes alphabetically for
> improved readability. There's no need for the configfs header to come
> last.

Is it?  People seem to have all kinds of weird opinions, but I don't
think any ordering really makes sense.  What does make sense it dropping
the pointless empty line, so I've folded that into the next patch.

Re: [PATCH 05/12] samples: configfs: improve alignment of broken lines

[Christoph Hellwig]

On Thu, Sep 24, 2020 at 02:45:19PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Improve the coding style by correctly aligning broken lines where
> possible.

NAK, this actually makes the code much worse.  Two-tab indents are
technically superior as they avoid pointlessly long line, and avoid
continuous re-indenting when prototypes change even just slightly.

Re: [PATCH 09/12] samples: configfs: prefer sizeof(*var) to sizeof(struct var_type)

[Christoph Hellwig]

On Thu, Sep 24, 2020 at 02:45:23PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> It's better to use the size of the actual variable than its type when
> allocating memory. This also has the benefit of avoiding a line break
> here.

Either style has up an downside.  The variable based on tracks type
changes automatically, but on the other hand leads to lots of bugs
where people forget the *.  I'd rather not just change from one to
the other pointlessly.

Re: [PATCH 00/12] samples: configfs: refactor the configfs sample code

[Christoph Hellwig]

On Tue, Oct 06, 2020 at 12:40:00PM +0200, Bartosz Golaszewski wrote:
> Any comments on this? Can this still go into v5.10?

I've applied the ones that seems sensible.

Re: [PATCH 02/12] samples: configfs: order includes alphabetically

[Bartosz Golaszewski]

On Wed, Oct 7, 2020 at 3:43 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Sep 24, 2020 at 02:45:16PM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > The preferred coding style is to order all includes alphabetically for
> > improved readability. There's no need for the configfs header to come
> > last.
>
> Is it?  People seem to have all kinds of weird opinions, but I don't
> think any ordering really makes sense.  What does make sense it dropping
> the pointless empty line, so I've folded that into the next patch.

This is not just a baseless opinion, keeping headers sorted clearly
has an advantage: you more easily avoid duplicating includes, you see
right away if a header is already included or not. Many maintainers
will require ordering in new patches.

It's your call but it's better code with not much effort.

Bartosz

Re: [PATCH 02/12] samples: configfs: order includes alphabetically

[Christoph Hellwig]

On Thu, Oct 08, 2020 at 03:23:11PM +0200, Bartosz Golaszewski wrote:
> On Wed, Oct 7, 2020 at 3:43 PM Christoph Hellwig <hch@lst.de> wrote:
> >
> > On Thu, Sep 24, 2020 at 02:45:16PM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > The preferred coding style is to order all includes alphabetically for
> > > improved readability. There's no need for the configfs header to come
> > > last.
> >
> > Is it?  People seem to have all kinds of weird opinions, but I don't
> > think any ordering really makes sense.  What does make sense it dropping
> > the pointless empty line, so I've folded that into the next patch.
> 
> This is not just a baseless opinion, keeping headers sorted clearly
> has an advantage: you more easily avoid duplicating includes, you see
> right away if a header is already included or not. Many maintainers
> will require ordering in new patches.

Various maintainers required all kind of crap.  I've not seen one
I deal with require alphabetic ordering, but I've seen the equally
insane reverse christmas tree.

> It's your call but it's better code with not much effort.

No, it is not in any quantifyable way better.  It is different and you
might prefer that, but in the end change it is just churn.