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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

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@linux.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.