Re: staging: media: tegra-video: Use common error handling code in tegra_vi_graph_parse_one()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Dan, Markus,

On Sat, 2 Mar 2024 11:40:26 +0100
Markus Elfring <Markus.Elfring@xxxxxx> 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@xxxxxxxxxxx>  
> >
> > 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@xxxxxxxxxx/

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





[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux