| Message ID | 20200819020503.3079-2-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bugfix and optimize for drivers/nvdimm | expand |
> The memory priv->bus_desc.provider_name allocated by kstrdup() is not > freed correctly. How do you think about to choose an imperative wording for a corresponding change description? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=18445bf405cb331117bc98427b1ba6f12418ad17#n151 Regards, Markus
On 8/19/2020 8:28 PM, Markus Elfring wrote: >> The memory priv->bus_desc.provider_name allocated by kstrdup() is not >> freed correctly. > > How do you think about to choose an imperative wording for > a corresponding change description? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=18445bf405cb331117bc98427b1ba6f12418ad17#n151 OK, thanks. I think I known what "imperative wording" means now. I will rewrite the descriptions. > > Regards, > Markus > >
On Wed, Aug 19, 2020 at 10:28 PM Markus Elfring <Markus.Elfring@web.de> wrote: > > > The memory priv->bus_desc.provider_name allocated by kstrdup() is not > > freed correctly. Personally I thought his commit message was perfectly fine. A little unorthodox, but it works. > How do you think about to choose an imperative wording for > a corresponding change description? ...but this! This is word salad. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=18445bf405cb331117bc98427b1ba6f12418ad17#n151 > > Regards, > Markus
On Wed, Aug 19, 2020 at 12:05 PM Zhen Lei <thunder.leizhen@huawei.com> wrote: > > The memory priv->bus_desc.provider_name allocated by kstrdup() is not > freed correctly. > > Fixes: 49bddc73d15c ("libnvdimm/of_pmem: Provide a unique name for bus provider") > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> Yep, that's a bug. Reviewed-by: Oliver O'Halloran <oohall@gmail.com> > --- > drivers/nvdimm/of_pmem.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c > index 10dbdcdfb9ce913..1292ffca7b2ecc0 100644 > --- a/drivers/nvdimm/of_pmem.c > +++ b/drivers/nvdimm/of_pmem.c > @@ -36,6 +36,7 @@ static int of_pmem_region_probe(struct platform_device *pdev) > > priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc); > if (!bus) { > + kfree(priv->bus_desc.provider_name); > kfree(priv); > return -ENODEV; > } > @@ -83,6 +84,7 @@ static int of_pmem_region_remove(struct platform_device *pdev) > struct of_pmem_private *priv = platform_get_drvdata(pdev); > > nvdimm_bus_unregister(priv->bus); > + kfree(priv->bus_desc.provider_name); > kfree(priv); > > return 0; > -- > 1.8.3 > >
On 8/19/2020 9:35 PM, Oliver O'Halloran wrote: > On Wed, Aug 19, 2020 at 10:28 PM Markus Elfring <Markus.Elfring@web.de> wrote: >> >>> The memory priv->bus_desc.provider_name allocated by kstrdup() is not >>> freed correctly. > > Personally I thought his commit message was perfectly fine. A little > unorthodox, but it works. > >> How do you think about to choose an imperative wording for >> a corresponding change description? > > ...but this! This is word salad. Talented students are trained by strict teacher. All of us is trying to make things better. > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=18445bf405cb331117bc98427b1ba6f12418ad17#n151 >> >> Regards, >> Markus > >
On 8/19/2020 9:37 PM, Oliver O'Halloran wrote: > On Wed, Aug 19, 2020 at 12:05 PM Zhen Lei <thunder.leizhen@huawei.com> wrote: >> >> The memory priv->bus_desc.provider_name allocated by kstrdup() is not >> freed correctly. >> >> Fixes: 49bddc73d15c ("libnvdimm/of_pmem: Provide a unique name for bus provider") >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > Yep, that's a bug. > > Reviewed-by: Oliver O'Halloran <oohall@gmail.com> Thanks for your review. > >> --- >> drivers/nvdimm/of_pmem.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c >> index 10dbdcdfb9ce913..1292ffca7b2ecc0 100644 >> --- a/drivers/nvdimm/of_pmem.c >> +++ b/drivers/nvdimm/of_pmem.c >> @@ -36,6 +36,7 @@ static int of_pmem_region_probe(struct platform_device *pdev) >> >> priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc); >> if (!bus) { >> + kfree(priv->bus_desc.provider_name); >> kfree(priv); >> return -ENODEV; >> } >> @@ -83,6 +84,7 @@ static int of_pmem_region_remove(struct platform_device *pdev) >> struct of_pmem_private *priv = platform_get_drvdata(pdev); >> >> nvdimm_bus_unregister(priv->bus); >> + kfree(priv->bus_desc.provider_name); >> kfree(priv); >> >> return 0; >> -- >> 1.8.3 >> >> > > . >
diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c index 10dbdcdfb9ce913..1292ffca7b2ecc0 100644 --- a/drivers/nvdimm/of_pmem.c +++ b/drivers/nvdimm/of_pmem.c @@ -36,6 +36,7 @@ static int of_pmem_region_probe(struct platform_device *pdev) priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc); if (!bus) { + kfree(priv->bus_desc.provider_name); kfree(priv); return -ENODEV; } @@ -83,6 +84,7 @@ static int of_pmem_region_remove(struct platform_device *pdev) struct of_pmem_private *priv = platform_get_drvdata(pdev); nvdimm_bus_unregister(priv->bus); + kfree(priv->bus_desc.provider_name); kfree(priv); return 0;
The memory priv->bus_desc.provider_name allocated by kstrdup() is not freed correctly. Fixes: 49bddc73d15c ("libnvdimm/of_pmem: Provide a unique name for bus provider") Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- drivers/nvdimm/of_pmem.c | 2 ++ 1 file changed, 2 insertions(+)