| Message ID | e78f346f-6196-157f-6b74-76e49c708ee0@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/VPD: Improve handling binary sysfs attribute | expand |
On Thu, Jan 07, 2021 at 10:48:15PM +0100, Heiner Kallweit wrote: > Since 104daa71b396 ("PCI: Determine actual VPD size on first access") > attribute size is set to 0 (unlimited). So let's remove this now > dead code. Doesn't the 104daa71b396 commit log say "attr->size == 0" means the size is unknown (not unlimited)? But I don't think vpd.c does anything at all with attr->size other than set it to zero. Is there some reason we can't remove the "attr->size = 0" assignment, too? Maybe the sysfs attribute code uses it? But I don't see vpd.c doing anything that would contribute to that. Sorry, I'm just puzzled. > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/pci/vpd.c | 14 -------------- > 1 file changed, 14 deletions(-) > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > index 7915d10f9..a3fd09105 100644 > --- a/drivers/pci/vpd.c > +++ b/drivers/pci/vpd.c > @@ -403,13 +403,6 @@ static ssize_t read_vpd_attr(struct file *filp, struct kobject *kobj, > { > struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj)); > > - if (bin_attr->size > 0) { > - if (off > bin_attr->size) > - count = 0; > - else if (count > bin_attr->size - off) > - count = bin_attr->size - off; > - } > - > return pci_read_vpd(dev, off, count, buf); > } > > @@ -419,13 +412,6 @@ static ssize_t write_vpd_attr(struct file *filp, struct kobject *kobj, > { > struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj)); > > - if (bin_attr->size > 0) { > - if (off > bin_attr->size) > - count = 0; > - else if (count > bin_attr->size - off) > - count = bin_attr->size - off; > - } > - > return pci_write_vpd(dev, off, count, buf); > } > > -- > 2.30.0 > >
On 03.02.2021 00:59, Bjorn Helgaas wrote: > On Thu, Jan 07, 2021 at 10:48:15PM +0100, Heiner Kallweit wrote: >> Since 104daa71b396 ("PCI: Determine actual VPD size on first access") >> attribute size is set to 0 (unlimited). So let's remove this now >> dead code. > > Doesn't the 104daa71b396 commit log say "attr->size == 0" means the > size is unknown (not unlimited)? > If attr->size == 0, then sysfs code will not check whether a read/write access is out of bounds. It expects that the respective driver returns an error or less than the requested data when reading beyond the actual size. And that's exactly what we do in the VPD code. > But I don't think vpd.c does anything at all with attr->size other > than set it to zero. Is there some reason we can't remove the > "attr->size = 0" assignment, too? > We could rely on zero-initialization of the allocated memory. But leaving this assignment makes clear that sysfs code doesn't have to check length restrictions. > Maybe the sysfs attribute code uses it? But I don't see vpd.c doing > anything that would contribute to that. > Yes, attr->size is used by sysfs code only. > Sorry, I'm just puzzled. > >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/pci/vpd.c | 14 -------------- >> 1 file changed, 14 deletions(-) >> >> diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c >> index 7915d10f9..a3fd09105 100644 >> --- a/drivers/pci/vpd.c >> +++ b/drivers/pci/vpd.c >> @@ -403,13 +403,6 @@ static ssize_t read_vpd_attr(struct file *filp, struct kobject *kobj, >> { >> struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj)); >> >> - if (bin_attr->size > 0) { >> - if (off > bin_attr->size) >> - count = 0; >> - else if (count > bin_attr->size - off) >> - count = bin_attr->size - off; >> - } >> - >> return pci_read_vpd(dev, off, count, buf); >> } >> >> @@ -419,13 +412,6 @@ static ssize_t write_vpd_attr(struct file *filp, struct kobject *kobj, >> { >> struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj)); >> >> - if (bin_attr->size > 0) { >> - if (off > bin_attr->size) >> - count = 0; >> - else if (count > bin_attr->size - off) >> - count = bin_attr->size - off; >> - } >> - >> return pci_write_vpd(dev, off, count, buf); >> } >> >> -- >> 2.30.0 >> >>
diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c index 7915d10f9..a3fd09105 100644 --- a/drivers/pci/vpd.c +++ b/drivers/pci/vpd.c @@ -403,13 +403,6 @@ static ssize_t read_vpd_attr(struct file *filp, struct kobject *kobj, { struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj)); - if (bin_attr->size > 0) { - if (off > bin_attr->size) - count = 0; - else if (count > bin_attr->size - off) - count = bin_attr->size - off; - } - return pci_read_vpd(dev, off, count, buf); } @@ -419,13 +412,6 @@ static ssize_t write_vpd_attr(struct file *filp, struct kobject *kobj, { struct pci_dev *dev = to_pci_dev(kobj_to_dev(kobj)); - if (bin_attr->size > 0) { - if (off > bin_attr->size) - count = 0; - else if (count > bin_attr->size - off) - count = bin_attr->size - off; - } - return pci_write_vpd(dev, off, count, buf); }
Since 104daa71b396 ("PCI: Determine actual VPD size on first access") attribute size is set to 0 (unlimited). So let's remove this now dead code. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/pci/vpd.c | 14 -------------- 1 file changed, 14 deletions(-)