On Sun, 30 Aug 2015, walter harms wrote: > > > Am 30.08.2015 20:05, schrieb Julia Lawall: > > Apply PTR_ERR to the value that was recently assigned. > > > > The semantic match that finds this problem is as follows: > > (http://coccinelle.lip6.fr/) > > > > // <smpl> > > @@ > > expression x,y; > > @@ > > > > if (IS_ERR(x) || ...) { > > ... when any > > when != IS_ERR(...) > > ( > > PTR_ERR(x) > > | > > * PTR_ERR(y) > > ) > > ... when any > > } > > // </smpl> > > > > Signed-off-by: Julia Lawall <Julia.Lawall@xxxxxxx> > > > > --- > > sound/soc/qcom/lpass-cpu.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/sound/soc/qcom/lpass-cpu.c b/sound/soc/qcom/lpass-cpu.c > > index 23f3d59..94beb99 100644 > > --- a/sound/soc/qcom/lpass-cpu.c > > +++ b/sound/soc/qcom/lpass-cpu.c > > @@ -438,7 +438,8 @@ int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev) > > if (IS_ERR(drvdata->mi2s_bit_clk[dai_id])) { > > dev_err(&pdev->dev, > > "%s() error getting mi2s-bit-clk: %ld\n", > > - __func__, PTR_ERR(drvdata->mi2s_bit_clk[i])); > > + __func__, > > + PTR_ERR(drvdata->mi2s_bit_clk[dai_id])); > > return PTR_ERR(drvdata->mi2s_bit_clk[dai_id]); > > } > > } > > > > just a note: > using a shorter name instead of drvdata->mi2s_bit_clk[dai_id] whould help to make the code > more readable (yes, the other code is alike). something like: > > struct clk *tmp = devm_clk_get(&pdev->dev,clk_name); Where do you suggest to put this? Maybe it would be reasonable to declare a variable struct clk *clk; at the top of the function, and then use that as a temporary variable for all three calls. However, now I see that the first call, unlike the other two doesn't cause a return from the function. if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) { dev_warn(&pdev->dev, "%s() error getting mi2s-osr-clk: %ld\n", __func__, PTR_ERR(drvdata->mi2s_osr_clk[dai_id])); } Is that intentional? thanks, julia > if (IS_ERR(tmp)) { > dev_err(&pdev->dev,"%s() error getting mi2s-bit-clk: %ld\n",__func__, PTR_ERR(tmp)); > return PTR_ERR(tmp); > } > drvdata->mi2s_bit_clk[dai_id]=tmp; > > > just one minor: > the dev_warn() just before says: " error getting mi2s-osr-clk" may be it should be "warnig ..." > That will make it more easy to rep for real error in a log. > > re, > wh > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
