* [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() @ 2021-04-28 6:32 Zhen Lei 2021-04-28 14:31 ` Vladimir Oltean 2021-05-17 7:04 ` Geert Uytterhoeven 0 siblings, 2 replies; 6+ messages in thread From: Zhen Lei @ 2021-04-28 6:32 UTC (permalink / raw) To: Greg Kroah-Hartman, linux-kernel; +Cc: Zhen Lei, Vladimir Oltean Ensure that all error handling branches print error information. In this way, when this function fails, the upper-layer functions can directly return an error code without missing debugging information. Otherwise, the error message will be printed redundantly or missing. Fixes: 35bd8c07db2c ("devres: keep both device name and resource name in pretty name") Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> --- lib/devres.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/devres.c b/lib/devres.c index 4679dbb1bf5f8a8..bdb06898a977701 100644 --- a/lib/devres.c +++ b/lib/devres.c @@ -157,8 +157,10 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, dev_name(dev), res->name); else pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL); - if (!pretty_name) + if (!pretty_name) { + dev_err(dev, "can't generate pretty name for resource %pR\n", res); return IOMEM_ERR_PTR(-ENOMEM); + } if (!devm_request_mem_region(dev, res->start, size, pretty_name)) { dev_err(dev, "can't request region for resource %pR\n", res); -- 2.26.0.106.g9fadedd ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() 2021-04-28 6:32 [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() Zhen Lei @ 2021-04-28 14:31 ` Vladimir Oltean 2021-05-17 7:04 ` Geert Uytterhoeven 1 sibling, 0 replies; 6+ messages in thread From: Vladimir Oltean @ 2021-04-28 14:31 UTC (permalink / raw) To: Zhen Lei; +Cc: Greg Kroah-Hartman, linux-kernel On Wed, Apr 28, 2021 at 02:32:03PM +0800, Zhen Lei wrote: > Ensure that all error handling branches print error information. In this > way, when this function fails, the upper-layer functions can directly > return an error code without missing debugging information. Otherwise, > the error message will be printed redundantly or missing. > > Fixes: 35bd8c07db2c ("devres: keep both device name and resource name in pretty name") > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > --- A Fixes: tag is probably a bit too drastic, but anyway: Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() 2021-04-28 6:32 [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() Zhen Lei 2021-04-28 14:31 ` Vladimir Oltean @ 2021-05-17 7:04 ` Geert Uytterhoeven 2021-06-07 7:10 ` Leizhen (ThunderTown) 1 sibling, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2021-05-17 7:04 UTC (permalink / raw) To: Zhen Lei; +Cc: Greg Kroah-Hartman, linux-kernel, Vladimir Oltean Hi Zhen, On Wed, Apr 28, 2021 at 8:33 AM Zhen Lei <thunder.leizhen@huawei.com> wrote: > Ensure that all error handling branches print error information. In this > way, when this function fails, the upper-layer functions can directly > return an error code without missing debugging information. Otherwise, > the error message will be printed redundantly or missing. > > Fixes: 35bd8c07db2c ("devres: keep both device name and resource name in pretty name") > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> Thanks for your patch, which is now commit 5c3e241f5246445d ("lib: devres: Add error information printing for __devm_ioremap_resource()") in driver-core-next. > --- a/lib/devres.c > +++ b/lib/devres.c > @@ -157,8 +157,10 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, > dev_name(dev), res->name); > else > pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL); > - if (!pretty_name) > + if (!pretty_name) { > + dev_err(dev, "can't generate pretty name for resource %pR\n", res); If the above fails, it is due to a memory allocation failure, and the memory allocation core will have already printed an error message. So there is no need to print a second message. Hence IMHO this commit should be reverted. > return IOMEM_ERR_PTR(-ENOMEM); > + } > > if (!devm_request_mem_region(dev, res->start, size, pretty_name)) { > dev_err(dev, "can't request region for resource %pR\n", res); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() 2021-05-17 7:04 ` Geert Uytterhoeven @ 2021-06-07 7:10 ` Leizhen (ThunderTown) 2021-06-07 8:15 ` Geert Uytterhoeven 0 siblings, 1 reply; 6+ messages in thread From: Leizhen (ThunderTown) @ 2021-06-07 7:10 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Greg Kroah-Hartman, linux-kernel, Vladimir Oltean On 2021/5/17 15:04, Geert Uytterhoeven wrote: > Hi Zhen, > > On Wed, Apr 28, 2021 at 8:33 AM Zhen Lei <thunder.leizhen@huawei.com> wrote: >> Ensure that all error handling branches print error information. In this >> way, when this function fails, the upper-layer functions can directly >> return an error code without missing debugging information. Otherwise, >> the error message will be printed redundantly or missing. >> >> Fixes: 35bd8c07db2c ("devres: keep both device name and resource name in pretty name") >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > Thanks for your patch, which is now commit 5c3e241f5246445d ("lib: > devres: Add error information printing for __devm_ioremap_resource()") > in driver-core-next. > >> --- a/lib/devres.c >> +++ b/lib/devres.c >> @@ -157,8 +157,10 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, >> dev_name(dev), res->name); >> else >> pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL); >> - if (!pretty_name) >> + if (!pretty_name) { >> + dev_err(dev, "can't generate pretty name for resource %pR\n", res); > > If the above fails, it is due to a memory allocation failure, and the > memory allocation core will have already printed an error message. > So there is no need to print a second message. > Hence IMHO this commit should be reverted. If the memory allocation fails, only the 'size' is printed, but the start address of the 'res' is not printed. So the printing here is not repeated. The start address is the more critical help information. > >> return IOMEM_ERR_PTR(-ENOMEM); >> + } >> >> if (!devm_request_mem_region(dev, res->start, size, pretty_name)) { >> dev_err(dev, "can't request region for resource %pR\n", res); > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() 2021-06-07 7:10 ` Leizhen (ThunderTown) @ 2021-06-07 8:15 ` Geert Uytterhoeven 2021-06-07 8:33 ` Leizhen (ThunderTown) 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2021-06-07 8:15 UTC (permalink / raw) To: Leizhen (ThunderTown); +Cc: Greg Kroah-Hartman, linux-kernel, Vladimir Oltean Hi Leizhen, On Mon, Jun 7, 2021 at 9:11 AM Leizhen (ThunderTown) <thunder.leizhen@huawei.com> wrote: > On 2021/5/17 15:04, Geert Uytterhoeven wrote: > > On Wed, Apr 28, 2021 at 8:33 AM Zhen Lei <thunder.leizhen@huawei.com> wrote: > >> Ensure that all error handling branches print error information. In this > >> way, when this function fails, the upper-layer functions can directly > >> return an error code without missing debugging information. Otherwise, > >> the error message will be printed redundantly or missing. > >> > >> Fixes: 35bd8c07db2c ("devres: keep both device name and resource name in pretty name") > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > > > Thanks for your patch, which is now commit 5c3e241f5246445d ("lib: > > devres: Add error information printing for __devm_ioremap_resource()") > > in driver-core-next. > > > >> --- a/lib/devres.c > >> +++ b/lib/devres.c > >> @@ -157,8 +157,10 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, > >> dev_name(dev), res->name); > >> else > >> pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL); > >> - if (!pretty_name) > >> + if (!pretty_name) { > >> + dev_err(dev, "can't generate pretty name for resource %pR\n", res); > > > > If the above fails, it is due to a memory allocation failure, and the > > memory allocation core will have already printed an error message. > > So there is no need to print a second message. > > Hence IMHO this commit should be reverted. > > If the memory allocation fails, only the 'size' is printed, but the start address of > the 'res' is not printed. So the printing here is not repeated. The start address is > the more critical help information. Does that actually matter? What can the user or developer do to fix this problem, with the additional knowledge of the resource's start address? > >> return IOMEM_ERR_PTR(-ENOMEM); > >> + } > >> > >> if (!devm_request_mem_region(dev, res->start, size, pretty_name)) { > >> dev_err(dev, "can't request region for resource %pR\n", res); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() 2021-06-07 8:15 ` Geert Uytterhoeven @ 2021-06-07 8:33 ` Leizhen (ThunderTown) 0 siblings, 0 replies; 6+ messages in thread From: Leizhen (ThunderTown) @ 2021-06-07 8:33 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Greg Kroah-Hartman, linux-kernel, Vladimir Oltean On 2021/6/7 16:15, Geert Uytterhoeven wrote: > Hi Leizhen, > > On Mon, Jun 7, 2021 at 9:11 AM Leizhen (ThunderTown) > <thunder.leizhen@huawei.com> wrote: >> On 2021/5/17 15:04, Geert Uytterhoeven wrote: >>> On Wed, Apr 28, 2021 at 8:33 AM Zhen Lei <thunder.leizhen@huawei.com> wrote: >>>> Ensure that all error handling branches print error information. In this >>>> way, when this function fails, the upper-layer functions can directly >>>> return an error code without missing debugging information. Otherwise, >>>> the error message will be printed redundantly or missing. >>>> >>>> Fixes: 35bd8c07db2c ("devres: keep both device name and resource name in pretty name") >>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >>> >>> Thanks for your patch, which is now commit 5c3e241f5246445d ("lib: >>> devres: Add error information printing for __devm_ioremap_resource()") >>> in driver-core-next. >>> >>>> --- a/lib/devres.c >>>> +++ b/lib/devres.c >>>> @@ -157,8 +157,10 @@ __devm_ioremap_resource(struct device *dev, const struct resource *res, >>>> dev_name(dev), res->name); >>>> else >>>> pretty_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL); >>>> - if (!pretty_name) >>>> + if (!pretty_name) { >>>> + dev_err(dev, "can't generate pretty name for resource %pR\n", res); >>> >>> If the above fails, it is due to a memory allocation failure, and the >>> memory allocation core will have already printed an error message. >>> So there is no need to print a second message. >>> Hence IMHO this commit should be reverted. >> >> If the memory allocation fails, only the 'size' is printed, but the start address of >> the 'res' is not printed. So the printing here is not repeated. The start address is >> the more critical help information. > > Does that actually matter? > What can the user or developer do to fix this problem, with the > additional knowledge of the resource's start address? Oh, you're right. > >>>> return IOMEM_ERR_PTR(-ENOMEM); >>>> + } >>>> >>>> if (!devm_request_mem_region(dev, res->start, size, pretty_name)) { >>>> dev_err(dev, "can't request region for resource %pR\n", res); > > Gr{oetje,eeting}s, > > Geert > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-07 8:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-28 6:32 [PATCH 1/1] lib: devres: Add error information printing for __devm_ioremap_resource() Zhen Lei 2021-04-28 14:31 ` Vladimir Oltean 2021-05-17 7:04 ` Geert Uytterhoeven 2021-06-07 7:10 ` Leizhen (ThunderTown) 2021-06-07 8:15 ` Geert Uytterhoeven 2021-06-07 8:33 ` Leizhen (ThunderTown)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.