Messages in this thread | | | Subject | Re: [v3] [media] Use common error handling code in 19 functions | From | SF Markus Elfring <> | Date | Sat, 5 May 2018 09:53:00 +0200 |
| |
> @@ -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
| |