linux-safety.lists.elisa.tech archive mirror
 help / color / mirror / Atom feed
* [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO
@ 2020-10-26 18:00 Sudip Mukherjee
  2020-10-27  5:36 ` [linux-safety] " Lukas Bulwahn
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2020-10-26 18:00 UTC (permalink / raw)
  To: John Stultz, Arnd Bergmann, Greg Kroah-Hartman
  Cc: linux-kernel, linux-safety, Sudip Mukherjee

Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
doing 'return 0'.

Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
---
 drivers/misc/hisi_hikey_usb.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
index cc93569e601c..989d7d129469 100644
--- a/drivers/misc/hisi_hikey_usb.c
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -168,10 +168,7 @@ static int hisi_hikey_usb_parse_kirin970(struct platform_device *pdev,
 
 	hisi_hikey_usb->reset = devm_gpiod_get(&pdev->dev, "hub_reset_en_gpio",
 					       GPIOD_OUT_HIGH);
-	if (IS_ERR(hisi_hikey_usb->reset))
-		return PTR_ERR(hisi_hikey_usb->reset);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(hisi_hikey_usb->reset);
 }
 
 static int hisi_hikey_usb_probe(struct platform_device *pdev)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [linux-safety] [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO
  2020-10-26 18:00 [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO Sudip Mukherjee
@ 2020-10-27  5:36 ` Lukas Bulwahn
  2020-10-27 10:08   ` Sudip Mukherjee
  2020-10-27  6:59 ` Lukas Bulwahn
  2020-11-03  6:11 ` John Stultz
  2 siblings, 1 reply; 6+ messages in thread
From: Lukas Bulwahn @ 2020-10-27  5:36 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: linux-safety



On Mon, 26 Oct 2020, Sudip Mukherjee wrote:

> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> doing 'return 0'.
> 
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> ---
>  drivers/misc/hisi_hikey_usb.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
> index cc93569e601c..989d7d129469 100644
> --- a/drivers/misc/hisi_hikey_usb.c
> +++ b/drivers/misc/hisi_hikey_usb.c
> @@ -168,10 +168,7 @@ static int hisi_hikey_usb_parse_kirin970(struct platform_device *pdev,
>  
>  	hisi_hikey_usb->reset = devm_gpiod_get(&pdev->dev, "hub_reset_en_gpio",
>  					       GPIOD_OUT_HIGH);
> -	if (IS_ERR(hisi_hikey_usb->reset))
> -		return PTR_ERR(hisi_hikey_usb->reset);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(hisi_hikey_usb->reset);
>  }
>

(Just among us, not with the kernel maintainers...)

I think such patches should be also CCed to kernel-janitors.

For the next patch: could you include the specific coccinelle rule upon 
which you reacted?

I think in the very long-term future, it would be nice if we can somehow 
link a finding from a specific tool (with some kind of URI) in the commit 
message or so.

E.g., https://findings.kernel.org/coccinelle/somehash/ and then it shows 
the specific finding you looked at in a specific version and links to
the reported findings in other runs and versions.

(more realistically, it would probably end up in kcidb or so...)

That would be a nice tool to allow good traceability of the work at low 
additional effort.

Well just a random thought... What do you think?

Lukas

>  static int hisi_hikey_usb_probe(struct platform_device *pdev)
> -- 
> 2.11.0
> 
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [linux-safety] [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO
  2020-10-26 18:00 [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO Sudip Mukherjee
  2020-10-27  5:36 ` [linux-safety] " Lukas Bulwahn
@ 2020-10-27  6:59 ` Lukas Bulwahn
  2020-11-03  6:11 ` John Stultz
  2 siblings, 0 replies; 6+ messages in thread
From: Lukas Bulwahn @ 2020-10-27  6:59 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: John Stultz, Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	linux-safety



On Mon, 26 Oct 2020, Sudip Mukherjee wrote:

> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> doing 'return 0'.
> 
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

A nice small local clean up.

Here is my test report:

Build requires HISI_HIKEY_USB in config:

Symbol: HISI_HIKEY_USB [=n]
Type: tristate
Defined at drivers/misc/Kconfig:459
Prompt: USB GPIO Hub on HiSilicon Hikey 960/970 Platform
Depends on: (OF [=n] && GPIOLIB [=n] || COMPILE_TEST [=n]) && 
USB_ROLE_SWITCH [=n]

Build Preparation:

make defconfig
./scripts/config -e CONFIG_OF
./scripts/config -e CONFIG_GPIOLIB
./scripts/config -e CONFIG_USB_ROLE_SWITCH
./scripts/config -e CONFIG_HISI_HIKEY_USB
make olddefconfig

Build on next-20201027:

make
md5sum ./drivers/misc/hisi_hikey_usb.o
770db9ff0c1ca193eda2207a28deee4a  ./drivers/misc/hisi_hikey_usb.o
objdump -d ./drivers/misc/hisi_hikey_usb.o > hisi_hikey_usb.next-20201027.objdump

Build with patch on next-20201027:

make clean && make
md5sum ./drivers/misc/hisi_hikey_usb.o
770db9ff0c1ca193eda2207a28deee4a  ./drivers/misc/hisi_hikey_usb.o

objdump -d ./drivers/misc/hisi_hikey_usb.o > hisi_hikey_usb.cleanup-on-next-20201027.objdump

Compare objdump:

diff hisi_hikey_usb.next-20201027.objdump hisi_hikey_usb.cleanup-on-next-20201027.objdump

Summary:

'{md5sum,objdump -d} ./drivers/misc/hisi_hikey_usb.o' are identical before 
and after patch.

So, no functional change and no change in object code.

Tested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>

> ---
>  drivers/misc/hisi_hikey_usb.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
> index cc93569e601c..989d7d129469 100644
> --- a/drivers/misc/hisi_hikey_usb.c
> +++ b/drivers/misc/hisi_hikey_usb.c
> @@ -168,10 +168,7 @@ static int hisi_hikey_usb_parse_kirin970(struct platform_device *pdev,
>  
>  	hisi_hikey_usb->reset = devm_gpiod_get(&pdev->dev, "hub_reset_en_gpio",
>  					       GPIOD_OUT_HIGH);
> -	if (IS_ERR(hisi_hikey_usb->reset))
> -		return PTR_ERR(hisi_hikey_usb->reset);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(hisi_hikey_usb->reset);
>  }
>  
>  static int hisi_hikey_usb_probe(struct platform_device *pdev)
> -- 
> 2.11.0
> 
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [linux-safety] [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO
  2020-10-27  5:36 ` [linux-safety] " Lukas Bulwahn
@ 2020-10-27 10:08   ` Sudip Mukherjee
  2020-10-27 10:23     ` Lukas Bulwahn
  0 siblings, 1 reply; 6+ messages in thread
From: Sudip Mukherjee @ 2020-10-27 10:08 UTC (permalink / raw)
  To: Lukas Bulwahn; +Cc: linux-safety

Hi Lukas,

On 27/10/2020 05:36, Lukas Bulwahn wrote:
> 
> 
> On Mon, 26 Oct 2020, Sudip Mukherjee wrote:
> 
>> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
>> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
>> doing 'return 0'.
>>
>> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
>> ---

<snip>

> 
> I think in the very long-term future, it would be nice if we can somehow 
> link a finding from a specific tool (with some kind of URI) in the commit 
> message or so.

I was thinking something almost similar, and was wondering if I can add
the tool name with some tags and then it will be possible to generate
some stats later about which tool fixed more things. I will send out a
mail later today to lkml proposing that we use such a tag.

I think adding URI is not possible until we have a persistent link to
these reports. The report that I generate can only have last 4 days
report, so the URI will not be valid after 4th day.



-- 
Regards
Sudip

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [linux-safety] [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO
  2020-10-27 10:08   ` Sudip Mukherjee
@ 2020-10-27 10:23     ` Lukas Bulwahn
  0 siblings, 0 replies; 6+ messages in thread
From: Lukas Bulwahn @ 2020-10-27 10:23 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: Lukas Bulwahn, linux-safety



On Tue, 27 Oct 2020, Sudip Mukherjee wrote:

> Hi Lukas,
> 
> On 27/10/2020 05:36, Lukas Bulwahn wrote:
> > 
> > 
> > On Mon, 26 Oct 2020, Sudip Mukherjee wrote:
> > 
> >> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> >> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> >> doing 'return 0'.
> >>
> >> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> >> ---
> 
> <snip>
> 
> > 
> > I think in the very long-term future, it would be nice if we can somehow 
> > link a finding from a specific tool (with some kind of URI) in the commit 
> > message or so.
> 
> I was thinking something almost similar, and was wondering if I can add
> the tool name with some tags and then it will be possible to generate
> some stats later about which tool fixed more things. I will send out a
> mail later today to lkml proposing that we use such a tag.
>
> I think adding URI is not possible until we have a persistent link to
> these reports. The report that I generate can only have last 4 days
> report, so the URI will not be valid after 4th day.
>

I would prefer to first include the reports into kernel ci db and maybe 
then we understand much better how to make those persistent links.

Lukas

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO
  2020-10-26 18:00 [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO Sudip Mukherjee
  2020-10-27  5:36 ` [linux-safety] " Lukas Bulwahn
  2020-10-27  6:59 ` Lukas Bulwahn
@ 2020-11-03  6:11 ` John Stultz
  2 siblings, 0 replies; 6+ messages in thread
From: John Stultz @ 2020-11-03  6:11 UTC (permalink / raw)
  To: Sudip Mukherjee; +Cc: Arnd Bergmann, Greg Kroah-Hartman, lkml, linux-safety

On Mon, Oct 26, 2020 at 11:02 AM Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
>
> Coccinelle suggested using PTR_ERR_OR_ZERO() and looking at the code,
> we can use PTR_ERR_OR_ZERO() instead of checking IS_ERR() and then
> doing 'return 0'.
>
> Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

Thanks for sending this!

Acked-by: John Stultz <john.stultz@linaro.org>

thanks
-john

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-03  6:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-26 18:00 [PATCH] misc: hisi_hikey_usb: use PTR_ERR_OR_ZERO Sudip Mukherjee
2020-10-27  5:36 ` [linux-safety] " Lukas Bulwahn
2020-10-27 10:08   ` Sudip Mukherjee
2020-10-27 10:23     ` Lukas Bulwahn
2020-10-27  6:59 ` Lukas Bulwahn
2020-11-03  6:11 ` John Stultz

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).