linux-kernel - Re: [PATCH v6 4/4] power: supply: core: add UAPI to discover currently used extensions

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <lbtleh6bihwxbj4nlj24wddqvhr7srrkyq5osi4oc6kaxq225n@asu5qkwxe6zc>
Date: Wed, 18 Dec 2024 23:46:23 +0100
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Nathan Chancellor <nathan@...nel.org>, Armin Wolf <W_Armin@....de>, 
	Hans de Goede <hdegoede@...hat.com>, Benson Leung <bleung@...omium.org>, 
	Guenter Roeck <groeck@...omium.org>, linux-kernel@...r.kernel.org, chrome-platform@...ts.linux.dev, 
	linux-pm@...r.kernel.org
Subject: Re: [PATCH v6 4/4] power: supply: core: add UAPI to discover
 currently used extensions

Hi,

On Wed, Dec 18, 2024 at 11:16:16PM +0100, Thomas Weißschuh wrote:
> On 2024-12-18 23:11:35+0100, Sebastian Reichel wrote:
> > On Wed, Dec 18, 2024 at 09:29:31PM +0100, Thomas Weißschuh wrote:
> > > On 2024-12-18 12:52:29-0700, Nathan Chancellor wrote:
> > > > I am seeing a build failure in certain configurations because
> > > > power_supply_extension_group is only declared under a CONFIG_SYSFS ifdef
> > > > but this code can be built without CONFIG_SYSFS.
> > > 
> > > Thanks for the report.
> > > 
> > > >   $ echo 'CONFIG_EXPERT=y
> > > >   CONFIG_SYSFS=n' >allno.config
> > > > 
> > > >   $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux- KCONFIG_ALLCONFIG=1 mrproper allnoconfig drivers/power/supply/power_supply_core.o
> > > >   drivers/power/supply/power_supply_core.c: In function 'power_supply_register_extension':
> > > >   drivers/power/supply/power_supply_core.c:1389:55: error: 'power_supply_extension_group' undeclared (first use in this function); did you mean 'power_supply_attr_groups'?
> > > >    1389 |         ret = sysfs_add_link_to_group(&psy->dev.kobj, power_supply_extension_group.name,
> > > >         |                                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >         |                                                       power_supply_attr_groups
> > > >   drivers/power/supply/power_supply_core.c:1389:55: note: each undeclared identifier is reported only once for each function it appears in
> > > >   drivers/power/supply/power_supply_core.c: In function 'power_supply_unregister_extension':
> > > >   drivers/power/supply/power_supply_core.c:1419:54: error: 'power_supply_extension_group' undeclared (first use in this function); did you mean 'power_supply_attr_groups'?
> > > >    1419 |                                                      power_supply_extension_group.name,
> > > >         |                                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >         |                                                      power_supply_attr_groups
> > > 
> > > The reproducer doesn't actually enable CONFIG_POWER_SUPPLY, when I use it
> > > I get a whole array of errors.
> > > 
> > > > Should the declaration be moved out from the ifdef or is there some
> > > > other solution I am not seeing?
> > > 
> > > This, inline constants or a #define.
> > > 
> > > Sebastian, do you want me to send a patch?
> > 
> > Yes, please send a patch. I suppose a define next to the NULL define
> > for power_supply_attr_groups should be good enough and consistent
> > with existing handling of this problem in the subsystem. 
> 
> That doesn't work because of the usage of the member "name" of the
> symbol power_supply_extension_group.

Right. Maybe make the power_supply_extension_group static to
power_supply_sysfs.c and instead offer power_supply_add_extension_sysfs()
as a wrapper to sysfs_add_link_to_group(). Then power_supply.h can
provide a dummy for !CONFIG_SYSFS making this similar to the
power_supply_add_hwmon_sysfs() handling.

-- Sebastian

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