Re: [v3] [media] Use common error handling code in 19 functions

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

 



> @@ -656,18 +656,18 @@  static int dvb_dmxdev_start_feed(struct dmxdev *dmxdev,
>  	tsfeed->priv = filter;
>  
>  	ret = tsfeed->set(tsfeed, feed->pid, ts_type, ts_pes, timeout);
> -	if (ret < 0) {
> -		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto release_feed;
>  
>  	ret = tsfeed->start_filtering(tsfeed);
> -	if (ret < 0) {
> -		dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto release_feed;
>  
>  	return 0;
> +
> +release_feed:
> +	dmxdev->demux->release_ts_feed(dmxdev->demux, tsfeed);
> +	return ret;
>  }
> 
> There's *nothing* wrong with the above. It works fine,

I can agree to this view in principle according to the required control flow.


> avoids goto

How does this wording fit to information from the section
“7) Centralized exiting of functions” in the document “coding-style.rst”?


> and probably even produce the same code, as gcc will likely optimize it.

Would you like to clarify the current situation around supported
software optimisations any more?


> It is also easier to review, as the error handling is closer
> to the code.

Do we stumble on different coding style preferences once more?


> On the other hand, there's nothing wrong on taking the approach
> you're proposing.

Thanks for another bit of positive feedback.


> In the end, using goto or not on error handling like the above is 
> a matter of personal taste - and taste changes with time

Do Linux guidelines need any adjustments?


> and with developer. I really don't have time to keep reviewing patches
> that are just churning the code just due to someone's personal taste.

I tried to apply another general source code transformation pattern.


> I'm pretty sure if I start accepting things like that,
> someone else would be on some future doing patches just reverting it,
> and I would be likely having to apply them too.

Why?

I hope also that the source code can be kept consistent to some degree.


> So, except if the patch is really fixing something - e.g. a broken
> error handling code, I'll just ignore such patches and mark as
> rejected without further notice/comments from now on.

I would find such a communication style questionable.
Do you distinguish between bug fixes and possible corrections for
other error categories (or software weaknesses)?

Regards,
Markus



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux