linux-kernel - Re: [PATCH v2] regulator: event: Add regulator netlink event support

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: <CABqG17jRbqcw=QioyZTPFTidUDiPDgBFbm45vfvG2Sc5TOznvw@mail.gmail.com>
Date:   Mon, 27 Nov 2023 16:07:45 +0530
From:   Naresh Solanki <naresh.solanki@...ements.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Liam Girdwood <lgirdwood@...il.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] regulator: event: Add regulator netlink event support

Hi Mark,


On Thu, 23 Nov 2023 at 18:27, Mark Brown <broonie@...nel.org> wrote:
>
> On Tue, Nov 21, 2023 at 10:13:30AM +0000, Naresh Solanki wrote:
>
> > This commit introduces netlink event support to the regulator subsystem.
>
> This looks a lot nicer, there's some feedback below but nothing
> maissively substantial.
>
> > +#ifdef CONFIG_NET
>
> This is essentially the entire file - it's probably better to just put
> the stub functions in the header and do the building with Kconfig.
Sure.
As I understood, Will add an option REGULATOR_NETLINK to include the
event.c file during build.
>
> > +static unsigned int reg_event_seqnum;
> > +struct reg_genl_event {
> > +     char reg_name[15];
> > +     u64 event;
> > +};
>
> Given that this is going to get sent to userspace shouldn't it be in a
> uapi header?  Some of the other event types don't seem to do this
> though...  that might be an issue with those examples though.   We'll
> also make the event numbers into uapi so they should go in a uapi header
> as well.
Will update next revision to include uapi header.
>
> I'm also not clear on where the 15 byte limit comes from.
I felt 15 characters would be sufficient for regulator names.
Would need your inputs to make sure here.
>
> > +EXPORT_SYMBOL(reg_generate_netlink_event);
>
> Everything else in regulator is EXPORT_SYMBOL_GPL() so this should be
> too - though given that the only caller is _notifier_call_chain() which
> is in the core does it need to be exported at all?  I can't see a case
> for anything calling it independently of that.
Agree. The current scope is limited to regulator core so will remove the
export.



Regards,
Naresh