* [PATCH 0/2] change PTR_ERR argument @ 2015-08-30 18:05 Julia Lawall 2015-08-30 18:05 ` [PATCH 1/2] ASoC: qcom: " Julia Lawall 2015-08-30 18:05 ` [PATCH 2/2] spi: spi-ep93xx: " Julia Lawall 0 siblings, 2 replies; 14+ messages in thread From: Julia Lawall @ 2015-08-30 18:05 UTC (permalink / raw) To: alsa-devel; +Cc: kernel-janitors, Mark Brown, linux-kernel, linux-spi Apply PTR_ERR to the value that was recently assigned. --- var/julia/linuxcopy/drivers/spi/spi-ep93xx.c | 3 ++- var/julia/linuxcopy/sound/soc/qcom/lpass-cpu.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-08-30 18:05 [PATCH 0/2] change PTR_ERR argument Julia Lawall @ 2015-08-30 18:05 ` Julia Lawall 2015-08-30 18:54 ` walter harms ` (2 more replies) 2015-08-30 18:05 ` [PATCH 2/2] spi: spi-ep93xx: " Julia Lawall 1 sibling, 3 replies; 14+ messages in thread From: Julia Lawall @ 2015-08-30 18:05 UTC (permalink / raw) To: Patrick Lai Cc: kernel-janitors, Banajit Goswami, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel 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@lip6.fr> --- 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]); } } ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-08-30 18:05 ` [PATCH 1/2] ASoC: qcom: " Julia Lawall @ 2015-08-30 18:54 ` walter harms 2015-08-30 19:54 ` Julia Lawall 2015-09-03 21:33 ` Kenneth Westfield 2015-09-03 21:19 ` Kenneth Westfield 2015-09-14 18:04 ` Mark Brown 2 siblings, 2 replies; 14+ messages in thread From: walter harms @ 2015-08-30 18:54 UTC (permalink / raw) To: Julia Lawall Cc: Patrick Lai, kernel-janitors, Banajit Goswami, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel 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@lip6.fr> > > --- > 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); 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-08-30 18:54 ` walter harms @ 2015-08-30 19:54 ` Julia Lawall 2015-09-03 21:36 ` [alsa-devel] " Kenneth Westfield 2015-09-03 21:33 ` Kenneth Westfield 1 sibling, 1 reply; 14+ messages in thread From: Julia Lawall @ 2015-08-30 19:54 UTC (permalink / raw) To: walter harms Cc: Patrick Lai, kernel-janitors, Banajit Goswami, Liam Girdwood, Mark Brown, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel 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@lip6.fr> > > > > --- > > 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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-08-30 19:54 ` Julia Lawall @ 2015-09-03 21:36 ` Kenneth Westfield 0 siblings, 0 replies; 14+ messages in thread From: Kenneth Westfield @ 2015-09-03 21:36 UTC (permalink / raw) To: Julia Lawall Cc: walter harms, alsa-devel, Banajit Goswami, linux-kernel, Patrick Lai, Takashi Iwai, kernel-janitors, Liam Girdwood, Mark Brown On Sun, Aug 30, 2015 at 12:54:15PM -0700, Julia Lawall wrote: > On Sun, 30 Aug 2015, walter harms wrote: > > Am 30.08.2015 20:05, schrieb Julia Lawall: > > > 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? Yes, that was intentional as the presense of the OSR clock in the DT node is optional. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-08-30 18:54 ` walter harms 2015-08-30 19:54 ` Julia Lawall @ 2015-09-03 21:33 ` Kenneth Westfield 1 sibling, 0 replies; 14+ messages in thread From: Kenneth Westfield @ 2015-09-03 21:33 UTC (permalink / raw) To: walter harms Cc: Julia Lawall, alsa-devel, Banajit Goswami, linux-kernel, Patrick Lai, Takashi Iwai, kernel-janitors, Liam Girdwood, Mark Brown On Sun, Aug 30, 2015 at 11:54:33AM -0700, walter harms wrote: > Am 30.08.2015 20:05, schrieb Julia Lawall: > > 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); > > 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; Yes, it would make the code more readable. > > > 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. "error [gs]etting" could be re-phrased to "could not [gs]et". The term "error" was not meant to indicate the log level, but evidently, can cause some confusion for someone reading the logs. -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [alsa-devel] [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-08-30 18:05 ` [PATCH 1/2] ASoC: qcom: " Julia Lawall 2015-08-30 18:54 ` walter harms @ 2015-09-03 21:19 ` Kenneth Westfield 2015-09-14 18:04 ` Mark Brown 2 siblings, 0 replies; 14+ messages in thread From: Kenneth Westfield @ 2015-09-03 21:19 UTC (permalink / raw) To: Julia Lawall Cc: Patrick Lai, alsa-devel, Banajit Goswami, linux-kernel, kernel-janitors, Takashi Iwai, Liam Girdwood, Mark Brown, Srinivas Kandagatla On Sun, Aug 30, 2015 at 11:05:10AM -0700, Julia Lawall wrote: > 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@lip6.fr> > > --- The patch itself looks good. Thanks. Acked-by: Kenneth Westfield <kwestfie@codeaurora.org> -- Kenneth Westfield Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-08-30 18:05 ` [PATCH 1/2] ASoC: qcom: " Julia Lawall 2015-08-30 18:54 ` walter harms 2015-09-03 21:19 ` Kenneth Westfield @ 2015-09-14 18:04 ` Mark Brown 2015-09-17 8:46 ` Julia Lawall 2015-09-17 8:47 ` Julia Lawall 2 siblings, 2 replies; 14+ messages in thread From: Mark Brown @ 2015-09-14 18:04 UTC (permalink / raw) To: Julia Lawall Cc: Patrick Lai, kernel-janitors, Banajit Goswami, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 461 bytes --] On Sun, Aug 30, 2015 at 08:05:10PM +0200, Julia Lawall wrote: > Apply PTR_ERR to the value that was recently assigned. > > The semantic match that finds this problem is as follows: > (http://coccinelle.lip6.fr/) I have no idea what's going on with this stuff without spending more time than it looks like should need, there's a moderately big thread and some patches posted in the middle of it. Can you repost whatever the current state is please? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-09-14 18:04 ` Mark Brown @ 2015-09-17 8:46 ` Julia Lawall 2015-09-17 9:21 ` Mark Brown 2015-09-17 8:47 ` Julia Lawall 1 sibling, 1 reply; 14+ messages in thread From: Julia Lawall @ 2015-09-17 8:46 UTC (permalink / raw) To: Mark Brown Cc: Julia Lawall, Patrick Lai, kernel-janitors, Banajit Goswami, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel On Mon, 14 Sep 2015, Mark Brown wrote: > On Sun, Aug 30, 2015 at 08:05:10PM +0200, Julia Lawall wrote: > > Apply PTR_ERR to the value that was recently assigned. > > > > The semantic match that finds this problem is as follows: > > (http://coccinelle.lip6.fr/) > > I have no idea what's going on with this stuff without spending more > time than it looks like should need, there's a moderately big thread and > some patches posted in the middle of it. Can you repost whatever the > current state is please? The discussion was about introducing a temporary variable to simplify the code. But that makes a lot of changes, so I think it would be better to just apply the original bug fixing patch as is, and then the cleanup could be applied on top of that. I will submit the original patch again. julia ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-09-17 8:46 ` Julia Lawall @ 2015-09-17 9:21 ` Mark Brown 0 siblings, 0 replies; 14+ messages in thread From: Mark Brown @ 2015-09-17 9:21 UTC (permalink / raw) To: Julia Lawall Cc: Patrick Lai, kernel-janitors, Banajit Goswami, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 388 bytes --] On Thu, Sep 17, 2015 at 10:46:16AM +0200, Julia Lawall wrote: > The discussion was about introducing a temporary variable to simplify the > code. But that makes a lot of changes, so I think it would be better to > just apply the original bug fixing patch as is, and then the cleanup could > be applied on top of that. I will submit the original patch again. OK, makes sense - thanks. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 473 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] ASoC: qcom: change PTR_ERR argument 2015-09-14 18:04 ` Mark Brown 2015-09-17 8:46 ` Julia Lawall @ 2015-09-17 8:47 ` Julia Lawall 1 sibling, 0 replies; 14+ messages in thread From: Julia Lawall @ 2015-09-17 8:47 UTC (permalink / raw) To: Mark Brown Cc: Julia Lawall, Patrick Lai, kernel-janitors, Banajit Goswami, Liam Girdwood, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-kernel 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@lip6.fr> --- 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]); } } ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] spi: spi-ep93xx: change PTR_ERR argument 2015-08-30 18:05 [PATCH 0/2] change PTR_ERR argument Julia Lawall 2015-08-30 18:05 ` [PATCH 1/2] ASoC: qcom: " Julia Lawall @ 2015-08-30 18:05 ` Julia Lawall 2015-08-30 18:31 ` walter harms 1 sibling, 1 reply; 14+ messages in thread From: Julia Lawall @ 2015-08-30 18:05 UTC (permalink / raw) To: Mark Brown; +Cc: kernel-janitors, linux-spi, linux-kernel 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@lip6.fr> --- drivers/spi/spi-ep93xx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c index bb00be8..1a7696c 100644 --- a/drivers/spi/spi-ep93xx.c +++ b/drivers/spi/spi-ep93xx.c @@ -567,7 +567,8 @@ static void ep93xx_spi_dma_transfer(struct ep93xx_spi *espi) txd = ep93xx_spi_dma_prepare(espi, DMA_MEM_TO_DEV); if (IS_ERR(txd)) { ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM); - dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", PTR_ERR(rxd)); + dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", + PTR_ERR(txd)); msg->status = PTR_ERR(txd); return; } ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] spi: spi-ep93xx: change PTR_ERR argument 2015-08-30 18:05 ` [PATCH 2/2] spi: spi-ep93xx: " Julia Lawall @ 2015-08-30 18:31 ` walter harms 2015-08-30 20:10 ` [PATCH 2/2 v2] spi: spi-ep93xx: fix PTR_ERR problem Julia Lawall 0 siblings, 1 reply; 14+ messages in thread From: walter harms @ 2015-08-30 18:31 UTC (permalink / raw) To: Julia Lawall; +Cc: Mark Brown, kernel-janitors, linux-spi, linux-kernel 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@lip6.fr> > > --- > drivers/spi/spi-ep93xx.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c > index bb00be8..1a7696c 100644 > --- a/drivers/spi/spi-ep93xx.c > +++ b/drivers/spi/spi-ep93xx.c > @@ -567,7 +567,8 @@ static void ep93xx_spi_dma_transfer(struct ep93xx_spi *espi) > txd = ep93xx_spi_dma_prepare(espi, DMA_MEM_TO_DEV); > if (IS_ERR(txd)) { > ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM); > - dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", PTR_ERR(rxd)); > + dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", > + PTR_ERR(txd)); > msg->status = PTR_ERR(txd); > return; > } > I improve readability i would suggest: ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM); msg->status = PTR_ERR(txd); dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n",msg->status): return; just my 2 cents, re, wh > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2 v2] spi: spi-ep93xx: fix PTR_ERR problem 2015-08-30 18:31 ` walter harms @ 2015-08-30 20:10 ` Julia Lawall 0 siblings, 0 replies; 14+ messages in thread From: Julia Lawall @ 2015-08-30 20:10 UTC (permalink / raw) To: walter harms; +Cc: Mark Brown, kernel-janitors, linux-spi, linux-kernel Move initialization of msg->status up over the call to dev_err, in both calls to ep93xx_spi_dma_prepare, and change the reference in the call to dev_err to msg->status, to both fix the wrong argument to PTR_ERR in the second case and to make the dev_err line a little shorter. This required furthermore replacing %ld by %d, since msg->status is an integer. The semantic match that finds the PTR_ERR 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> Reorganizations at the suggestion of Walter Harms. Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr> --- v2: Reorganize the code, to solve the problem in a way that makes the resulting code simpler. drivers/spi/spi-ep93xx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-ep93xx.c b/drivers/spi/spi-ep93xx.c index bb00be8..73d0df6 100644 --- a/drivers/spi/spi-ep93xx.c +++ b/drivers/spi/spi-ep93xx.c @@ -559,16 +559,16 @@ static void ep93xx_spi_dma_transfer(struct ep93xx_spi *espi) rxd = ep93xx_spi_dma_prepare(espi, DMA_DEV_TO_MEM); if (IS_ERR(rxd)) { - dev_err(&espi->pdev->dev, "DMA RX failed: %ld\n", PTR_ERR(rxd)); msg->status = PTR_ERR(rxd); + dev_err(&espi->pdev->dev, "DMA RX failed: %d\n", msg->status); return; } txd = ep93xx_spi_dma_prepare(espi, DMA_MEM_TO_DEV); if (IS_ERR(txd)) { ep93xx_spi_dma_finish(espi, DMA_DEV_TO_MEM); - dev_err(&espi->pdev->dev, "DMA TX failed: %ld\n", PTR_ERR(rxd)); msg->status = PTR_ERR(txd); + dev_err(&espi->pdev->dev, "DMA TX failed: %d\n", msg->status); return; } ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-09-17 9:21 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-08-30 18:05 [PATCH 0/2] change PTR_ERR argument Julia Lawall 2015-08-30 18:05 ` [PATCH 1/2] ASoC: qcom: " Julia Lawall 2015-08-30 18:54 ` walter harms 2015-08-30 19:54 ` Julia Lawall 2015-09-03 21:36 ` [alsa-devel] " Kenneth Westfield 2015-09-03 21:33 ` Kenneth Westfield 2015-09-03 21:19 ` Kenneth Westfield 2015-09-14 18:04 ` Mark Brown 2015-09-17 8:46 ` Julia Lawall 2015-09-17 9:21 ` Mark Brown 2015-09-17 8:47 ` Julia Lawall 2015-08-30 18:05 ` [PATCH 2/2] spi: spi-ep93xx: " Julia Lawall 2015-08-30 18:31 ` walter harms 2015-08-30 20:10 ` [PATCH 2/2 v2] spi: spi-ep93xx: fix PTR_ERR problem Julia Lawall
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).