Re: [v3] [media] Use common error handling code in 19 functions
From: SF Markus Elfring
Date: Sat May 05 2018 - 03:55:59 EST
> @@ -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