All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
@ 2020-04-05  9:30 ` Markus Elfring
  0 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2020-04-05  9:30 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Paul Cercueil
  Cc: kernel-janitors, LKML, Tang Bin

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 5 Apr 2020 11:25:30 +0200

The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 9dfe7cb530e1..06ca752b76ee 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 	}

 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		dev_err(dev, "Failed to get platform irq");
+	if (irq < 0)
 		return irq;
-	}

 	if (soc_info->needs_dev_clk) {
 		priv->lcd_clk = devm_clk_get(dev, "lcd");
--
2.26.0

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
@ 2020-04-05  9:30 ` Markus Elfring
  0 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2020-04-05  9:30 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Paul Cercueil
  Cc: kernel-janitors, LKML, Tang Bin

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 5 Apr 2020 11:25:30 +0200

The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 9dfe7cb530e1..06ca752b76ee 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 	}

 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		dev_err(dev, "Failed to get platform irq");
+	if (irq < 0)
 		return irq;
-	}

 	if (soc_info->needs_dev_clk) {
 		priv->lcd_clk = devm_clk_get(dev, "lcd");
--
2.26.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
@ 2020-04-05  9:30 ` Markus Elfring
  0 siblings, 0 replies; 16+ messages in thread
From: Markus Elfring @ 2020-04-05  9:30 UTC (permalink / raw)
  To: dri-devel, Daniel Vetter, David Airlie, Paul Cercueil
  Cc: LKML, kernel-janitors, Tang Bin

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 5 Apr 2020 11:25:30 +0200

The function “platform_get_irq” can log an error already.
Thus omit a redundant message for the exception handling in the
calling function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
index 9dfe7cb530e1..06ca752b76ee 100644
--- a/drivers/gpu/drm/ingenic/ingenic-drm.c
+++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
@@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device *pdev)
 	}

 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		dev_err(dev, "Failed to get platform irq");
+	if (irq < 0)
 		return irq;
-	}

 	if (soc_info->needs_dev_clk) {
 		priv->lcd_clk = devm_clk_get(dev, "lcd");
--
2.26.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
  (?)
@ 2020-04-05 11:49   ` Paul Cercueil
  -1 siblings, 0 replies; 16+ messages in thread
From: Paul Cercueil @ 2020-04-05 11:49 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Tang Bin, David Airlie, kernel-janitors, LKML, dri-devel

Hi,

Le dim. 5 avril 2020 à 11:30, Markus Elfring <Markus.Elfring@web.de> a 
écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 5 Apr 2020 11:25:30 +0200
> 
> The function “platform_get_irq” can log an error already.
> Thus omit a redundant message for the exception handling in the
> calling function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index 9dfe7cb530e1..06ca752b76ee 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct 
> platform_device *pdev)
>  	}
> 
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(dev, "Failed to get platform irq");
> +	if (irq < 0)
>  		return irq;
> -	}
> 
>  	if (soc_info->needs_dev_clk) {
>  		priv->lcd_clk = devm_clk_get(dev, "lcd");
> --
> 2.26.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
@ 2020-04-05 11:49   ` Paul Cercueil
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Cercueil @ 2020-04-05 11:49 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Tang Bin, David Airlie, kernel-janitors, LKML, dri-devel

Hi,

Le dim. 5 avril 2020 à 11:30, Markus Elfring <Markus.Elfring@web.de> a 
écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 5 Apr 2020 11:25:30 +0200
> 
> The function “platform_get_irq” can log an error already.
> Thus omit a redundant message for the exception handling in the
> calling function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index 9dfe7cb530e1..06ca752b76ee 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct 
> platform_device *pdev)
>  	}
> 
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(dev, "Failed to get platform irq");
> +	if (irq < 0)
>  		return irq;
> -	}
> 
>  	if (soc_info->needs_dev_clk) {
>  		priv->lcd_clk = devm_clk_get(dev, "lcd");
> --
> 2.26.0
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
@ 2020-04-05 11:49   ` Paul Cercueil
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Cercueil @ 2020-04-05 11:49 UTC (permalink / raw)
  To: Markus Elfring
  Cc: dri-devel, Daniel Vetter, David Airlie, LKML, kernel-janitors,
	Tang Bin

Hi,

Le dim. 5 avril 2020 à 11:30, Markus Elfring <Markus.Elfring@web.de> a 
écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 5 Apr 2020 11:25:30 +0200
> 
> The function “platform_get_irq” can log an error already.
> Thus omit a redundant message for the exception handling in the
> calling function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index 9dfe7cb530e1..06ca752b76ee 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct 
> platform_device *pdev)
>  	}
> 
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(dev, "Failed to get platform irq");
> +	if (irq < 0)
>  		return irq;
> -	}
> 
>  	if (soc_info->needs_dev_clk) {
>  		priv->lcd_clk = devm_clk_get(dev, "lcd");
> --
> 2.26.0
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
                   ` (2 preceding siblings ...)
  (?)
@ 2020-04-05 17:27 ` Christophe JAILLET
  -1 siblings, 0 replies; 16+ messages in thread
From: Christophe JAILLET @ 2020-04-05 17:27 UTC (permalink / raw)
  To: kernel-janitors

Le 05/04/2020 à 11:30, Markus Elfring a écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 5 Apr 2020 11:25:30 +0200
>
> The function “platform_get_irq” can log an error already.
> Thus omit a redundant message for the exception handling in the
> calling function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index 9dfe7cb530e1..06ca752b76ee 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device *pdev)
>   	}
>
>   	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(dev, "Failed to get platform irq");

Some 'dev_err' or equivalent functions sometimes don't have a trailing 
'\n'. (just like here)
Do you think that it worth fixing? Or is it to low level value?

According to a few grep, there seems to be quite a lot of them to fix.

Julia, can 'coccinelle' be used for that?

CJ

> +	if (irq < 0)
>   		return irq;
> -	}
>
>   	if (soc_info->needs_dev_clk) {
>   		priv->lcd_clk = devm_clk_get(dev, "lcd");
> --
> 2.26.0
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
                   ` (3 preceding siblings ...)
  (?)
@ 2020-04-05 17:54 ` Julia Lawall
  -1 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2020-04-05 17:54 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 2092 bytes --]



On Sun, 5 Apr 2020, Christophe JAILLET wrote:

> Le 05/04/2020 à 11:30, Markus Elfring a écrit :
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Sun, 5 Apr 2020 11:25:30 +0200
> >
> > The function “platform_get_irq” can log an error already.
> > Thus omit a redundant message for the exception handling in the
> > calling function.
> >
> > This issue was detected by using the Coccinelle software.
> >
> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > ---
> >   drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
> > b/drivers/gpu/drm/ingenic/ingenic-drm.c
> > index 9dfe7cb530e1..06ca752b76ee 100644
> > --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> > +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> > @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device
> > *pdev)
> >   	}
> >
> >   	irq = platform_get_irq(pdev, 0);
> > -	if (irq < 0) {
> > -		dev_err(dev, "Failed to get platform irq");
>
> Some 'dev_err' or equivalent functions sometimes don't have a trailing '\n'.
> (just like here)
> Do you think that it worth fixing? Or is it to low level value?
>
> According to a few grep, there seems to be quite a lot of them to fix.
>
> Julia, can 'coccinelle' be used for that?

Yes, it should be possible by writing some script code.

Something like

@initialize:python@
@@
... // define check_for_missing_nl (returning a boolean) and add_newline

@r@
constant str : script:python() { check_for_missing_nl str };
expression e;
@@

dev_err(e,str,...)

@script:python s@
str << r.str;
strnl;
@@

coccinelle.strnl = add_newline str

@@
constant r.str;
identifier s.strnl;
@@

dev_err(e,
- str
+ strnl
  ,...)

One would have to be a bit careful in add_newline to keep the "s even
though the code pretends that strnl is an identifier.

julia

>
> CJ
>
> > +	if (irq < 0)
> >   		return irq;
> > -	}
> >
> >   	if (soc_info->needs_dev_clk) {
> >   		priv->lcd_clk = devm_clk_get(dev, "lcd");
> > --
> > 2.26.0
> >
> >
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
                   ` (4 preceding siblings ...)
  (?)
@ 2020-04-05 18:52 ` Christophe JAILLET
  -1 siblings, 0 replies; 16+ messages in thread
From: Christophe JAILLET @ 2020-04-05 18:52 UTC (permalink / raw)
  To: kernel-janitors

Le 05/04/2020 à 19:54, Julia Lawall a écrit :
>
> On Sun, 5 Apr 2020, Christophe JAILLET wrote:
>
>> Le 05/04/2020 à 11:30, Markus Elfring a écrit :
>>> From: Markus Elfring <elfring@users.sourceforge.net>
>>> Date: Sun, 5 Apr 2020 11:25:30 +0200
>>>
>>> The function “platform_get_irq” can log an error already.
>>> Thus omit a redundant message for the exception handling in the
>>> calling function.
>>>
>>> This issue was detected by using the Coccinelle software.
>>>
>>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>>> ---
>>>    drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> b/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> index 9dfe7cb530e1..06ca752b76ee 100644
>>> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device
>>> *pdev)
>>>    	}
>>>
>>>    	irq = platform_get_irq(pdev, 0);
>>> -	if (irq < 0) {
>>> -		dev_err(dev, "Failed to get platform irq");
>> Some 'dev_err' or equivalent functions sometimes don't have a trailing '\n'.
>> (just like here)
>> Do you think that it worth fixing? Or is it to low level value?
>>
>> According to a few grep, there seems to be quite a lot of them to fix.
>>
>> Julia, can 'coccinelle' be used for that?
> Yes, it should be possible by writing some script code.
>
> Something like
>
> @initialize:python@
> @@
> ... // define check_for_missing_nl (returning a boolean) and add_newline
>
> @r@
> constant str : script:python() { check_for_missing_nl str };
> expression e;
> @@
>
> dev_err(e,str,...)
>
> @script:python s@
> str << r.str;
> strnl;
> @@
>
> coccinelle.strnl = add_newline str
>
> @@
> constant r.str;
> identifier s.strnl;
> @@
>
> dev_err(e,
> - str
> + strnl
>    ,...)
>
> One would have to be a bit careful in add_newline to keep the "s even
> though the code pretends that strnl is an identifier.
>
> julia

Hi Julia,
thx for the sample, I'll give it a try.

CJ

>> CJ
>>
>>> +	if (irq < 0)
>>>    		return irq;
>>> -	}
>>>
>>>    	if (soc_info->needs_dev_clk) {
>>>    		priv->lcd_clk = devm_clk_get(dev, "lcd");
>>> --
>>> 2.26.0
>>>
>>>
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
                   ` (5 preceding siblings ...)
  (?)
@ 2020-04-06 17:48 ` Christophe JAILLET
  -1 siblings, 0 replies; 16+ messages in thread
From: Christophe JAILLET @ 2020-04-06 17:48 UTC (permalink / raw)
  To: kernel-janitors

Le 05/04/2020 à 19:54, Julia Lawall a écrit :
>
> On Sun, 5 Apr 2020, Christophe JAILLET wrote:
>
>> Le 05/04/2020 à 11:30, Markus Elfring a écrit :
>>> From: Markus Elfring <elfring@users.sourceforge.net>
>>> Date: Sun, 5 Apr 2020 11:25:30 +0200
>>>
>>> The function “platform_get_irq” can log an error already.
>>> Thus omit a redundant message for the exception handling in the
>>> calling function.
>>>
>>> This issue was detected by using the Coccinelle software.
>>>
>>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>>> ---
>>>    drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>>>    1 file changed, 1 insertion(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> b/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> index 9dfe7cb530e1..06ca752b76ee 100644
>>> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
>>> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device
>>> *pdev)
>>>    	}
>>>
>>>    	irq = platform_get_irq(pdev, 0);
>>> -	if (irq < 0) {
>>> -		dev_err(dev, "Failed to get platform irq");
>> Some 'dev_err' or equivalent functions sometimes don't have a trailing '\n'.
>> (just like here)
>> Do you think that it worth fixing? Or is it to low level value?
>>
>> According to a few grep, there seems to be quite a lot of them to fix.
>>
>> Julia, can 'coccinelle' be used for that?
> Yes, it should be possible by writing some script code.
>
> Something like
>
> @initialize:python@
> @@
> ... // define check_for_missing_nl (returning a boolean) and add_newline
>
> @r@
> constant str : script:python() { check_for_missing_nl str };

I can not have this work.
According to my understanding of [1], this syntax is only allowed for 
'position'.

Nevertheless, I wrote another script (see below), which triggers ~2800 
times in ./drivers only.
Some are false positives, but most look valid.

Not sure that fixing this kind of stuff really make sense.

A better approach could be to teach ./checkpatch.pl, but I don't have 
the knowledge for that.


CJ

[1]: http://coccinelle.lip6.fr/docs/main_grammar.pdf (SmPL Grammar 1.0.8)

> expression e;
> @@
>
> dev_err(e,str,...)
>
> @script:python s@
> str << r.str;
> strnl;
> @@
>
> coccinelle.strnl = add_newline str
>
> @@
> constant r.str;
> identifier s.strnl;
> @@
>
> dev_err(e,
> - str
> + strnl
>    ,...)
>
> One would have to be a bit careful in add_newline to keep the "s even
> though the code pretends that strnl is an identifier.
>
> julia
>
>> CJ
>>
>>> +	if (irq < 0)
>>>    		return irq;
>>> -	}
>>>
>>>    	if (soc_info->needs_dev_clk) {
>>>    		priv->lcd_clk = devm_clk_get(dev, "lcd");
>>> --
>>> 2.26.0
>>>
>>>
> >

 >>>>>>>>>>>>>>>>>>>>>>>>>>>

@initialize:python@
@@

@r@
constant str;
expression e;
position p;
@@
(
         pr_emerg@p(str, ...)
|
         pr_alert@p(str, ...)
|
         pr_crit@p(str, ...)
|
         pr_err@p(str, ...)
|
         pr_warn@p(str, ...)
|
         pr_notice@p(str, ...)
|
         pr_info@p(str, ...)
|
         dev_emerg@p(e, str, ...)
|
         dev_alert@p(e, str, ...)
|
         dev_crit@p(e, str, ...)
|
         dev_err@p(e, str, ...)
|
         dev_warn@p(e, str, ...)
|
         dev_notice@p(e, str, ...)
|
         dev_info@p(e, str, ...)
)

@script:python depends on r@
str << r.str;
p << r.p;
@@
if not str.endswith("\\n\""):
     print(p[0].file + ":" + p[0].line + ": " + str)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
                   ` (6 preceding siblings ...)
  (?)
@ 2020-04-06 18:50 ` Julia Lawall
  -1 siblings, 0 replies; 16+ messages in thread
From: Julia Lawall @ 2020-04-06 18:50 UTC (permalink / raw)
  To: kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 4088 bytes --]



On Mon, 6 Apr 2020, Christophe JAILLET wrote:

> Le 05/04/2020 à 19:54, Julia Lawall a écrit :
> >
> > On Sun, 5 Apr 2020, Christophe JAILLET wrote:
> >
> > > Le 05/04/2020 à 11:30, Markus Elfring a écrit :
> > > > From: Markus Elfring <elfring@users.sourceforge.net>
> > > > Date: Sun, 5 Apr 2020 11:25:30 +0200
> > > >
> > > > The function “platform_get_irq” can log an error already.
> > > > Thus omit a redundant message for the exception handling in the
> > > > calling function.
> > > >
> > > > This issue was detected by using the Coccinelle software.
> > > >
> > > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> > > > ---
> > > >    drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
> > > >    1 file changed, 1 insertion(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c
> > > > b/drivers/gpu/drm/ingenic/ingenic-drm.c
> > > > index 9dfe7cb530e1..06ca752b76ee 100644
> > > > --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> > > > +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> > > > @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct platform_device
> > > > *pdev)
> > > >    	}
> > > >
> > > >    	irq = platform_get_irq(pdev, 0);
> > > > -	if (irq < 0) {
> > > > -		dev_err(dev, "Failed to get platform irq");
> > > Some 'dev_err' or equivalent functions sometimes don't have a trailing
> > > '\n'.
> > > (just like here)
> > > Do you think that it worth fixing? Or is it to low level value?
> > >
> > > According to a few grep, there seems to be quite a lot of them to fix.
> > >
> > > Julia, can 'coccinelle' be used for that?
> > Yes, it should be possible by writing some script code.
> >
> > Something like
> >
> > @initialize:python@
> > @@
> > ... // define check_for_missing_nl (returning a boolean) and add_newline
> >
> > @r@
> > constant str : script:python() { check_for_missing_nl str };
>
> I can not have this work.
> According to my understanding of [1], this syntax is only allowed for
> 'position'.

Maybe you have an older version of Coccinelle?  If you get the latest
version from github it should work.

>
> Nevertheless, I wrote another script (see below), which triggers ~2800 times
> in ./drivers only.
> Some are false positives, but most look valid.
>
> Not sure that fixing this kind of stuff really make sense.

Yes, it seems like a lot...

julia

>
> A better approach could be to teach ./checkpatch.pl, but I don't have the
> knowledge for that.
>
>
> CJ
>
> [1]: http://coccinelle.lip6.fr/docs/main_grammar.pdf (SmPL Grammar 1.0.8)
>
> > expression e;
> > @@
> >
> > dev_err(e,str,...)
> >
> > @script:python s@
> > str << r.str;
> > strnl;
> > @@
> >
> > coccinelle.strnl = add_newline str
> >
> > @@
> > constant r.str;
> > identifier s.strnl;
> > @@
> >
> > dev_err(e,
> > - str
> > + strnl
> >    ,...)
> >
> > One would have to be a bit careful in add_newline to keep the "s even
> > though the code pretends that strnl is an identifier.
> >
> > julia
> >
> > > CJ
> > >
> > > > +	if (irq < 0)
> > > >    		return irq;
> > > > -	}
> > > >
> > > >    	if (soc_info->needs_dev_clk) {
> > > >    		priv->lcd_clk = devm_clk_get(dev, "lcd");
> > > > --
> > > > 2.26.0
> > > >
> > > >
> > >
>
> >>>>>>>>>>>>>>>>>>>>>>>>>>>
>
> @initialize:python@
> @@
>
> @r@
> constant str;
> expression e;
> position p;
> @@
> (
>         pr_emerg@p(str, ...)
> |
>         pr_alert@p(str, ...)
> |
>         pr_crit@p(str, ...)
> |
>         pr_err@p(str, ...)
> |
>         pr_warn@p(str, ...)
> |
>         pr_notice@p(str, ...)
> |
>         pr_info@p(str, ...)
> |
>         dev_emerg@p(e, str, ...)
> |
>         dev_alert@p(e, str, ...)
> |
>         dev_crit@p(e, str, ...)
> |
>         dev_err@p(e, str, ...)
> |
>         dev_warn@p(e, str, ...)
> |
>         dev_notice@p(e, str, ...)
> |
>         dev_info@p(e, str, ...)
> )
>
> @script:python depends on r@
> str << r.str;
> p << r.p;
> @@
> if not str.endswith("\\n\""):
>     print(p[0].file + ":" + p[0].line + ": " + str)
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
  (?)
@ 2020-04-07 20:42   ` Paul Cercueil
  -1 siblings, 0 replies; 16+ messages in thread
From: Paul Cercueil @ 2020-04-07 20:42 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Tang Bin, David Airlie, kernel-janitors, LKML, dri-devel

Hi Markus,

Le dim. 5 avril 2020 à 11:30, Markus Elfring <Markus.Elfring@web.de> a 
écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 5 Apr 2020 11:25:30 +0200
> 
> The function “platform_get_irq” can log an error already.
> Thus omit a redundant message for the exception handling in the
> calling function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Applied to drm-misc-next, thanks.

-Paul

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index 9dfe7cb530e1..06ca752b76ee 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct 
> platform_device *pdev)
>  	}
> 
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(dev, "Failed to get platform irq");
> +	if (irq < 0)
>  		return irq;
> -	}
> 
>  	if (soc_info->needs_dev_clk) {
>  		priv->lcd_clk = devm_clk_get(dev, "lcd");
> --
> 2.26.0
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
@ 2020-04-07 20:42   ` Paul Cercueil
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Cercueil @ 2020-04-07 20:42 UTC (permalink / raw)
  To: Markus Elfring; +Cc: Tang Bin, David Airlie, kernel-janitors, LKML, dri-devel

Hi Markus,

Le dim. 5 avril 2020 à 11:30, Markus Elfring <Markus.Elfring@web.de> a 
écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 5 Apr 2020 11:25:30 +0200
> 
> The function “platform_get_irq” can log an error already.
> Thus omit a redundant message for the exception handling in the
> calling function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Applied to drm-misc-next, thanks.

-Paul

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index 9dfe7cb530e1..06ca752b76ee 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct 
> platform_device *pdev)
>  	}
> 
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(dev, "Failed to get platform irq");
> +	if (irq < 0)
>  		return irq;
> -	}
> 
>  	if (soc_info->needs_dev_clk) {
>  		priv->lcd_clk = devm_clk_get(dev, "lcd");
> --
> 2.26.0
> 


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
@ 2020-04-07 20:42   ` Paul Cercueil
  0 siblings, 0 replies; 16+ messages in thread
From: Paul Cercueil @ 2020-04-07 20:42 UTC (permalink / raw)
  To: Markus Elfring
  Cc: dri-devel, Daniel Vetter, David Airlie, LKML, kernel-janitors,
	Tang Bin

Hi Markus,

Le dim. 5 avril 2020 à 11:30, Markus Elfring <Markus.Elfring@web.de> a 
écrit :
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 5 Apr 2020 11:25:30 +0200
> 
> The function “platform_get_irq” can log an error already.
> Thus omit a redundant message for the exception handling in the
> calling function.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Applied to drm-misc-next, thanks.

-Paul

> ---
>  drivers/gpu/drm/ingenic/ingenic-drm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ingenic/ingenic-drm.c 
> b/drivers/gpu/drm/ingenic/ingenic-drm.c
> index 9dfe7cb530e1..06ca752b76ee 100644
> --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
> +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
> @@ -661,10 +661,8 @@ static int ingenic_drm_probe(struct 
> platform_device *pdev)
>  	}
> 
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(dev, "Failed to get platform irq");
> +	if (irq < 0)
>  		return irq;
> -	}
> 
>  	if (soc_info->needs_dev_clk) {
>  		priv->lcd_clk = devm_clk_get(dev, "lcd");
> --
> 2.26.0
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
                   ` (8 preceding siblings ...)
  (?)
@ 2020-04-08  9:42 ` Dan Carpenter
  -1 siblings, 0 replies; 16+ messages in thread
From: Dan Carpenter @ 2020-04-08  9:42 UTC (permalink / raw)
  To: kernel-janitors

On Mon, Apr 06, 2020 at 07:48:52PM +0200, Christophe JAILLET wrote:
> Nevertheless, I wrote another script (see below), which triggers ~2800 times
> in ./drivers only.
> Some are false positives, but most look valid.

I did a quick grep and you're right there are a ton of these!

> 
> Not sure that fixing this kind of stuff really make sense.
> 
> A better approach could be to teach ./checkpatch.pl, but I don't have the
> knowledge for that.

Yeah.  I think so too.  Here is what I tried.  $logFunctions ends up
catching too many functions like process_mcheck_info() so maybe we need
a stricter regex for that.

regards,
dan carpenter

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d64c67b67e3c..1bcfa9e18059 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5493,6 +5493,13 @@ sub process {
 			     "Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst\n" . $herecurr);
 		}
 
+# check for missing newline
+		if ($line =~ /\t$logFunctions\b.*$String[,)]/ && $rawline !~ /\\n/ &&
+			$rawline !~ / "[,)]/ &&
+			$line !~ /debugfs_/ && $line !~ /MODULE_/) {
+			WARN("MISSING_NEWLINE", "Missing newline?\n" . $herecurr);
+		}
+
 # Check for user-visible strings broken across lines, which breaks the ability
 # to grep for the string.  Make exceptions when the previous string ends in a
 # newline (multiple lines in one string constant) or '\t', '\r', ';', or '{'

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe()
  2020-04-05  9:30 ` Markus Elfring
                   ` (9 preceding siblings ...)
  (?)
@ 2020-04-08  9:49 ` Joe Perches
  -1 siblings, 0 replies; 16+ messages in thread
From: Joe Perches @ 2020-04-08  9:49 UTC (permalink / raw)
  To: kernel-janitors

On Wed, 2020-04-08 at 12:42 +0300, Dan Carpenter wrote:
> On Mon, Apr 06, 2020 at 07:48:52PM +0200, Christophe JAILLET wrote:
> > Nevertheless, I wrote another script (see below), which triggers ~2800 times
> > in ./drivers only.
> > Some are false positives, but most look valid.
> 
> I did a quick grep and you're right there are a ton of these!

Quite a lot of them use pr_cont too.

Anyway, try this:
https://lore.kernel.org/lkml/8617a6b94c0644bce1fd4ca77309d67a612e6300.camel@perches.com/

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-04-08  9:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-05  9:30 [PATCH] gpu/drm: ingenic: Delete an error message in ingenic_drm_probe() Markus Elfring
2020-04-05  9:30 ` Markus Elfring
2020-04-05  9:30 ` Markus Elfring
2020-04-05 11:49 ` Paul Cercueil
2020-04-05 11:49   ` Paul Cercueil
2020-04-05 11:49   ` Paul Cercueil
2020-04-05 17:27 ` Christophe JAILLET
2020-04-05 17:54 ` Julia Lawall
2020-04-05 18:52 ` Christophe JAILLET
2020-04-06 17:48 ` Christophe JAILLET
2020-04-06 18:50 ` Julia Lawall
2020-04-07 20:42 ` Paul Cercueil
2020-04-07 20:42   ` Paul Cercueil
2020-04-07 20:42   ` Paul Cercueil
2020-04-08  9:42 ` Dan Carpenter
2020-04-08  9:49 ` Joe Perches

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.