| Message ID | 20210603000112.703037-5-kw@linux.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions | expand |
[+cc Sebastian] On Thu, Jun 03, 2021 at 12:01:10AM +0000, Krzysztof Wilczyński wrote: > At the moment, when the value of the "devspec" sysfs object is read from > the user space there will be no newline present, and the utilities such > as the "cat" command won't display the result of the read correctly in > a shell, as the trailing newline is currently missing. > > To fix this, append a newline character in the show() function. There are a few other devspec_show() functions: arch/powerpc/platforms/pseries/ibmebus.c arch/powerpc/platforms/pseries/vio.c arch/sparc/kernel/vio.c drivers/usb/core/sysfs.c and they all include the newline. So I assume it's not likely that this minor user-visible change will break something. I did cc: Sebastian, since his dfc73e7acd99 ("PCI: Move Open Firmware devspec attribute to PCI common code") moved this code to pci-sysfs.c in the first place (it came from microblaze and powerpc code that *also* did not include the newline). I notice that pci-sysfs.c is the only one that returns 0 if of_node is NULL. Probably makes more sense than what the others do. I'm guessing the others would print "(null)" which doesn't seem quite right for a sysfs attribute. But pci-sysfs.c also goes to a little more work than necessary to look up of_node: struct pci_dev *pdev = to_pci_dev(dev); struct device_node *np = pci_device_to_OF_node(pdev); when I think this would be equivalent: struct device_node *np = dev->of_node; Some of the others do a similar dance with struct platform_device. Why'd I write all the above? I dunno; this looks good to me, no action required for you :) > Signed-off-by: Krzysztof Wilczyński <kw@linux.com> > Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > --- > drivers/pci/pci-sysfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c > index beb8d1f4fafe..5d63df7c1820 100644 > --- a/drivers/pci/pci-sysfs.c > +++ b/drivers/pci/pci-sysfs.c > @@ -537,7 +537,7 @@ static ssize_t devspec_show(struct device *dev, > > if (np == NULL) > return 0; > - return sysfs_emit(buf, "%pOF", np); > + return sysfs_emit(buf, "%pOF\n", np); > } > static DEVICE_ATTR_RO(devspec); > #endif > -- > 2.31.1 >
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index beb8d1f4fafe..5d63df7c1820 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -537,7 +537,7 @@ static ssize_t devspec_show(struct device *dev, if (np == NULL) return 0; - return sysfs_emit(buf, "%pOF", np); + return sysfs_emit(buf, "%pOF\n", np); } static DEVICE_ATTR_RO(devspec); #endif