| Message ID | 20220223191310.347669-6-krzysztof.kozlowski@canonical.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix broken usage of driver_override (and kfree of static memory) | expand |
In subject, to match drivers/pci/ convention, do something like: PCI: Use driver_set_override() instead of open-coding On Wed, Feb 23, 2022 at 08:13:04PM +0100, Krzysztof Kozlowski wrote: > Use a helper for seting driver_override to reduce amount of duplicated > code. s/seting/setting/ > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> > --- > drivers/pci/pci-sysfs.c | 24 ++++-------------------- > 1 file changed, 4 insertions(+), 20 deletions(-) > > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c > index 602f0fb0b007..16a163d4623e 100644 > --- a/drivers/pci/pci-sysfs.c > +++ b/drivers/pci/pci-sysfs.c > @@ -567,31 +567,15 @@ static ssize_t driver_override_store(struct device *dev, > const char *buf, size_t count) > { > struct pci_dev *pdev = to_pci_dev(dev); > - char *driver_override, *old, *cp; > + int ret; > > /* We need to keep extra room for a newline */ > if (count >= (PAGE_SIZE - 1)) > return -EINVAL; This check makes no sense in the new function. Michael alluded to this as well. > - driver_override = kstrndup(buf, count, GFP_KERNEL); > - if (!driver_override) > - return -ENOMEM; > - > - cp = strchr(driver_override, '\n'); > - if (cp) > - *cp = '\0'; > - > - device_lock(dev); > - old = pdev->driver_override; > - if (strlen(driver_override)) { > - pdev->driver_override = driver_override; > - } else { > - kfree(driver_override); > - pdev->driver_override = NULL; > - } > - device_unlock(dev); > - > - kfree(old); > + ret = driver_set_override(dev, &pdev->driver_override, buf); > + if (ret) > + return ret; > > return count; > } > -- > 2.32.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 23/02/2022 22:51, Bjorn Helgaas wrote: > In subject, to match drivers/pci/ convention, do something like: > > PCI: Use driver_set_override() instead of open-coding > > On Wed, Feb 23, 2022 at 08:13:04PM +0100, Krzysztof Kozlowski wrote: >> Use a helper for seting driver_override to reduce amount of duplicated >> code. > > s/seting/setting/ > >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> >> --- >> drivers/pci/pci-sysfs.c | 24 ++++-------------------- >> 1 file changed, 4 insertions(+), 20 deletions(-) >> >> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c >> index 602f0fb0b007..16a163d4623e 100644 >> --- a/drivers/pci/pci-sysfs.c >> +++ b/drivers/pci/pci-sysfs.c >> @@ -567,31 +567,15 @@ static ssize_t driver_override_store(struct device *dev, >> const char *buf, size_t count) >> { >> struct pci_dev *pdev = to_pci_dev(dev); >> - char *driver_override, *old, *cp; >> + int ret; >> >> /* We need to keep extra room for a newline */ >> if (count >= (PAGE_SIZE - 1)) >> return -EINVAL; > > This check makes no sense in the new function. Michael alluded to > this as well. I am not sure if I got your comment properly. You mean here: 1. Move this check to driver_set_override()? 2. Remove the check entirely? > >> - driver_override = kstrndup(buf, count, GFP_KERNEL); >> - if (!driver_override) >> - return -ENOMEM; >> - >> - cp = strchr(driver_override, '\n'); >> - if (cp) >> - *cp = '\0'; >> - >> - device_lock(dev); >> - old = pdev->driver_override; >> - if (strlen(driver_override)) { >> - pdev->driver_override = driver_override; >> - } else { >> - kfree(driver_override); >> - pdev->driver_override = NULL; >> - } >> - device_unlock(dev); >> - >> - kfree(old); >> + ret = driver_set_override(dev, &pdev->driver_override, buf); >> + if (ret) >> + return ret; >> >> return count; >> } >> -- >> 2.32.0 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel Best regards, Krzysztof
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c index 602f0fb0b007..16a163d4623e 100644 --- a/drivers/pci/pci-sysfs.c +++ b/drivers/pci/pci-sysfs.c @@ -567,31 +567,15 @@ static ssize_t driver_override_store(struct device *dev, const char *buf, size_t count) { struct pci_dev *pdev = to_pci_dev(dev); - char *driver_override, *old, *cp; + int ret; /* We need to keep extra room for a newline */ if (count >= (PAGE_SIZE - 1)) return -EINVAL; - driver_override = kstrndup(buf, count, GFP_KERNEL); - if (!driver_override) - return -ENOMEM; - - cp = strchr(driver_override, '\n'); - if (cp) - *cp = '\0'; - - device_lock(dev); - old = pdev->driver_override; - if (strlen(driver_override)) { - pdev->driver_override = driver_override; - } else { - kfree(driver_override); - pdev->driver_override = NULL; - } - device_unlock(dev); - - kfree(old); + ret = driver_set_override(dev, &pdev->driver_override, buf); + if (ret) + return ret; return count; }
Use a helper for seting driver_override to reduce amount of duplicated code. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com> --- drivers/pci/pci-sysfs.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)