Hi Kai
Thank you for feedback
thanks for the longer explanation. It's true we have a lot of code with for_each_xxx() loops, and loop logic where errors can occur. It seems the most common approach is to store the index and use for_each_xxx_rollback() macros to clean up in case error happens. This does lead to the problem of essentially duplicated logic e.g. for soc_pcm_close() and error handling of snd_pcm_open().
Thank you for understanding my headache. My opinion is that complex duplicated code never bring luck to us.
And yeah, it's also a bit error prone as the logic is not exactly the same. E.g. I'm not convinced this is quite right in soc_pcm_open():
» for_each_rtd_codec_dai(rtd, i, codec_dai) { » » ret = snd_soc_dai_startup(codec_dai, substream); » » if (ret < 0) { » » » dev_err(codec_dai->dev, » » » » "ASoC: can't open codec %s: %d\n", » » » » codec_dai->name, ret); » » » goto config_err; » » } ... config_err: » for_each_rtd_codec_dai(rtd, i, codec_dai) » » snd_soc_dai_shutdown(codec_dai, substream); » i = rtd->num_cpus;
Yeah indeed. I think this is just one of the BUG. I guess we can find similar issue everywhere.
Having a single cleanup path would be easier, but I think in the end it comes down to how cleanly you can track the opened state. It seems biggest issue is how to cleanly track the component-substream pairs. Ideally we'd have a dedicated state object to represent an opened component-substream pair, but that's not how the APIs are defined now. But something to that effect is needed.
Yeah, I can understand your concern, but not 100% yet. In my understanding, counting start vs stop is not enough but not so bad. If my understanding was correct, your concern is counting only is not enough, because wrong component-substream pair can be used, like this ?
start(substream-A); <= start(substream-B); start(substream-C);
stop(substream-Z); <= stop(substream-B); stop(substream-C);
But I wonder is it really happen ? If there is clear such case, yes, we need to consider component-substream pair list, somehow.
If you are worry about some kind of BUG, I guess we need to solve is such bug, not error handling method. But how about step-by-step approach (I like it :) ? At first, add counting method as 1st step. Indeed it is not enough, but we can cleanup error handling. If we found/noticed above not-enough-case, start to consider about component-substream pair list ?
Thank you for your help !! Best regards --- Kuninori Morimoto