linux-kernel - Re: [PATCH 4/7] pci: doe: Replace sysfs visibility macro

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] [day] [month] [year] [list]

Message-ID: <a8581a58-e994-ff5e-6bdc-ca9efe319da1@linux.intel.com>
Date: Sat, 26 Apr 2025 16:06:56 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: "David E. Box" <david.e.box@...ux.intel.com>
cc: corbet@....net, bhelgaas@...gle.com, kuurtb@...il.com, 
    Hans de Goede <hdegoede@...hat.com>, vkoul@...nel.org, 
    yung-chuan.liao@...ux.intel.com, pierre-louis.bossart@...ux.dev, 
    sanyog.r.kale@...el.com, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    "Rafael J. Wysocki" <rafael@...nel.org>, dakr@...nel.org, 
    dan.j.williams@...el.com, 
    Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
    linux-doc@...r.kernel.org, LKML <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 4/7] pci: doe: Replace sysfs visibility macro

On Fri, 25 Apr 2025, David E. Box wrote:

> On Fri, 2025-04-25 at 13:57 +0300, Ilpo Järvinen wrote:
> > On Wed, 23 Apr 2025, David E. Box wrote:
> > 
> > > Replace deprecated DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE() call with the new
> > > DEFINE_SYSFS_GROUP_VISIBILITY() helper for the pci_doe_features_sysfs group
> > > in drivers/pci/doe.c.
> > > 
> > > Signed-off-by: David E. Box <david.e.box@...ux.intel.com>
> > > ---
> > >  drivers/pci/doe.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> > > index aae9a8a00406..18b355506dc1 100644
> > > --- a/drivers/pci/doe.c
> > > +++ b/drivers/pci/doe.c
> > > @@ -119,7 +119,7 @@ static bool pci_doe_features_sysfs_group_visible(struct
> > > kobject *kobj)
> > >  
> > >  	return !xa_empty(&pdev->doe_mbs);
> > >  }
> > > -DEFINE_SIMPLE_SYSFS_GROUP_VISIBLE(pci_doe_features_sysfs)
> > > +DEFINE_SYSFS_GROUP_VISIBILITY(pci_doe_features_sysfs)
> > 
> > Hi David,
> > 
> > Is it intentional to not have semicolon at the end?
> 
> Hi Ilpo,
> 
> I was just doing a straight name swap and didn't not notice the lack of a
> semicolon. Of course, since DEFINE_SYSFS_GROUP_VISIBILITY() expands to a
> function definition, a trailing semicolon isn't necessary.
> 
> I suspect the issue is with the other instances where it was added, which makes
> the usage inconsistent. What would you suggest?

Hi,

When I saw that lack of semicolon, my first assumption was there's 
something special here that _requires_ leaving the semicolon out, which 
turned out untrue after an unnecessary roundtrip to read the macro. So IMO 
it would be better to have the semicolon there to tell the reader there's 
nothing of special interest here.

Also, you used semicolon in the example. :-)

-- 
 i.