linux-kernel - Re: [PATCH 1/7] sysfs: Rename attribute group visibility macros

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: <680993326d51e_71fe294c6@dwillia2-xfh.jf.intel.com.notmuch>
Date: Wed, 23 Apr 2025 18:26:10 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: "David E. Box" <david.e.box@...ux.intel.com>, <corbet@....net>,
	<bhelgaas@...gle.com>, <kuurtb@...il.com>, <hdegoede@...hat.com>,
	<ilpo.jarvinen@...ux.intel.com>, <vkoul@...nel.org>,
	<yung-chuan.liao@...ux.intel.com>, <pierre-louis.bossart@...ux.dev>,
	<sanyog.r.kale@...el.com>, <gregkh@...uxfoundation.org>, <rafael@...nel.org>,
	<dakr@...nel.org>, <dan.j.williams@...el.com>,
	<andriy.shevchenko@...ux.intel.com>
CC: <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-pci@...r.kernel.org>, <platform-driver-x86@...r.kernel.org>,
	<Dell.Client.Kernel@...l.com>, <linux-sound@...r.kernel.org>
Subject: Re: [PATCH 1/7] sysfs: Rename attribute group visibility macros

David E. Box wrote:
> Update the naming of several macros to provide clearer semantics for
> controlling group and attribute visibility per Dan Williams' suggestion.
> Also, add transitional aliases mapping the old macro names to the new ones
> so that driver code remains functional before changes are again made in a
> future macro encapsulation patch. This approach ensures that when the
> encapsulation work is applied, drivers will only need to be updated once
> without breaking compatibility.
> 
> Suggested-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: David E. Box <david.e.box@...ux.intel.com>
> ---
>  include/linux/sysfs.h | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 18f7e1fd093c..00dc88776f21 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -116,7 +116,7 @@ struct attribute_group {
>  #define SYSFS_GROUP_INVISIBLE	020000
>  
>  /*
> - * DEFINE_SYSFS_GROUP_VISIBLE(name):
> + * DEFINE_SYSFS_GROUP_COMBO_VISIBILITY(name):

This patch looks good, I just find myself wishing what "combo" means was
mentioned somewhere to clarify the distinction with the non-combo
flavor.

Something like:

@@ -123,6 +123,10 @@ struct attribute_group {
  *     This allows for static declaration of attribute_groups, and the
  *     simplification of attribute visibility lifetime that implies,
  *     without polluting sysfs with empty attribute directories.
+ *
+ *     "COMBO" implies that both the individual attribute
+ *     @name_attr_visible() and group @name_group_visible() helpers
+ *     must be defined.
  * Ex.
  *
  * static umode_t example_attr_visible(struct kobject *kobj,

>  /*
> - * DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name):
> + * DEFINE_SYSFS_GROUP_VISIBILITY(name):
>   *	A helper macro to pair with SYSFS_GROUP_VISIBLE() that like
> - *	DEFINE_SYSFS_GROUP_VISIBLE() controls group visibility, but does
> + *	DEFINE_SYSFS_GROUP_COMBO_VISIBILITY() controls group visibility, but does
>   *	not require the implementation of a per-attribute visibility
>   *	callback.

@@ -166,10 +170,10 @@ struct attribute_group {
 
 /*
  * DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(name):
- *     A helper macro to pair with SYSFS_GROUP_VISIBLE() that like
- *     DEFINE_SYSFS_GROUP_VISIBLE() controls group visibility, but does
- *     not require the implementation of a per-attribute visibility
- *     callback.
+ *     A helper macro to pair with SYSFS_GROUP_VISIBLE(). Unlike
+ *     DEFINE_SYSFS_GROUP_COMBO_VISIBILITY() only a single
+ *     @name_group_visible() helper needs to be defined.
+ *
  * Ex.
  *
  * static bool example_group_visible(struct kobject *kobj)

...and then that hopefully makes it clearer what the requirements are
because truth to be told even I forgot what "combo" meant.

With those clarifications you can add:

Reviewed-by: Dan Williams <dan.j.williams@...el.com>