Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 5 Mar 2024 16:24:27 +0100 | From | Luca Ceresoli <> | Subject | Re: staging: media: tegra-video: Use common error handling code in tegra_vi_graph_parse_one() |
| |
Hello Dan, Markus,
On Sat, 2 Mar 2024 11:40:26 +0100 Markus Elfring <Markus.Elfring@web.de> wrote:
> >>> Add a jump target so that a bit of exception handling can be better reused > >>> at the end of this function implementation. > … > >> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> > > > > These patches make the code worse.
This is of course a legitimate opinion. However Markus' patch implements what is recommended by the documentation and is in common use in the kernel code. A quick search found 73 occurrences in v6.8-rc7:
$ expr $(pcregrep -r -M ':\n\tfwnode_handle_put' drivers | wc -l) / 2 73 $
300+ are found for of_node_put().
> > If we're in the middle of a loop, > > then we should clean up the partial loop before doing the goto. > > Otherwise it creates a mess when we add a new allocation function after > > the end of the loop. > > How does such a feedback fit to another known information source? > > Section “7) Centralized exiting of functions” > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.8-rc6#n526 > > > Someone is going to add a _scoped() loop which uses cleanup.h magic to > > call _put automatically. This is a good option. > > I became also curious how scope-based resource management will influence > Linux coding styles further. > Will various collateral evolution become more interesting?
After some research I think I found what Dan means:
https://lore.kernel.org/all/20240225142714.286440-3-jic23@kernel.org/
After reading the above thread, I agree using *_scoped() macros will be a good improvement. It is not yet in mainline as of v6.8-rc7, but it is in linux-next. So I think despite being valid this patch might still be discarded because a better solution should be available in a few weeks.
Luca
-- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
| ![\](/images/icornerr.gif) |