cocci.inria.fr archive mirror
 help / color / mirror / Atom feed
* [cocci] Reconsidering repeated pointer checks with SmPL
@ 2023-03-15 17:36 Markus Elfring
  2023-03-16 20:07 ` [cocci] [PATCH 0/4] powerpc/4xx: Adjustments for four function implementations Markus Elfring
                   ` (30 more replies)
  0 siblings, 31 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-15 17:36 UTC (permalink / raw)
  To: cocci, kernel-janitors

Hello,

I tried the following SmPL script out also on the source files from
the software “Linux next-20230315”.

@display@
expression call, storage;
identifier target;
@@
*storage = call(...);
*if (!storage)
    goto target;
 ...
*target:
*if (storage)
 {
 ...
 }


79 source files were found where a check was performed for a failed function call
and an opposite check is immediately performed for the same variable.
I guess that pointer checks are repeated at these source code places.
I imagine that such code should be reconsidered once more and improved accordingly.

How do you think about to achieve any adjustments in this design direction?

Regards,
Markus


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

* [cocci] [PATCH 0/4] powerpc/4xx: Adjustments for four function implementations
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
@ 2023-03-16 20:07 ` Markus Elfring
  2023-03-25 15:30   ` [cocci] [PATCH v2 " Markus Elfring
       [not found] ` <0981dc33-95d0-4a1b-51d9-168907da99e6@web.de>
                   ` (29 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-16 20:07 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Benjamin Herrenschmidt,
	Christophe Leroy, Josh Boyer, Michael Ellerman, Nicholas Piggin,
	Stefan Roese
  Cc: cocci, LKML

Date: Thu, 16 Mar 2023 20:15:43 +0100

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Fix exception handling in ppc4xx_pciex_port_setup_hose()
  Fix exception handling in ppc4xx_probe_pcix_bridge()
  Fix exception handling in ppc4xx_probe_pci_bridge()
  Delete unnecessary variable initialisations in four functions

 arch/powerpc/platforms/4xx/pci.c | 69 ++++++++++++++------------------
 1 file changed, 31 insertions(+), 38 deletions(-)

--
2.39.2



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

* Re: [cocci] [PATCH] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()
       [not found] ` <0981dc33-95d0-4a1b-51d9-168907da99e6@web.de>
@ 2023-03-17 13:11   ` Nathan Lynch
  2023-03-17 14:20     ` [cocci] " Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Nathan Lynch @ 2023-03-17 13:11 UTC (permalink / raw)
  To: Markus Elfring
  Cc: cocci, LKML, kernel-janitors, linuxppc-dev, Christophe Leroy,
	Michael Ellerman, Nicholas Piggin, Paul Moore

Markus Elfring <Markus.Elfring@web.de> writes:
>
> The label “out_err” was used to jump to another pointer check despite of
> the detail in the implementation of the function “pSeries_reconfig_add_node”
> that it was determined already that the corresponding variable contained
> a null pointer (because of a failed function call in two cases).
>
> 1. Thus return directly after a call of the function “kzalloc” failed.
>
> 2. Use more appropriate labels instead.
>
> 3. Delete a redundant check.
>
> 4. Omit an explicit initialisation for the local variable “err”.
>
> This issue was detected by using the Coccinelle software.

Is there a correctness or safety issue here? The subject uses the word
"fix" but the commit message doesn't seem to identify one.

Can you share how Coccinelle is being invoked and its output?

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

* Re: [cocci] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()
  2023-03-17 13:11   ` [cocci] [PATCH] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node() Nathan Lynch
@ 2023-03-17 14:20     ` Markus Elfring
  2023-03-17 15:41       ` Nathan Lynch
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-17 14:20 UTC (permalink / raw)
  To: Nathan Lynch, linuxppc-dev, kernel-janitors
  Cc: cocci, LKML, Christophe Leroy, Michael Ellerman, Nicholas Piggin,
	Paul Moore

>> The label “out_err” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “pSeries_reconfig_add_node”
>> that it was determined already that the corresponding variable contained
>> a null pointer (because of a failed function call in two cases).
>>
>> 1. Thus return directly after a call of the function “kzalloc” failed.
>>
>> 2. Use more appropriate labels instead.
>>
>> 3. Delete a redundant check.
>>
>> 4. Omit an explicit initialisation for the local variable “err”.
>>
>> This issue was detected by using the Coccinelle software.
> Is there a correctness or safety issue here?

I got the impression that the application of only a single label like “out_err”
resulted in improvable implementation details.


> The subject uses the word "fix" but the commit message doesn't seem to identify one.

Can you find the proposed adjustments reasonable?


> Can you share how Coccinelle is being invoked and its output?

Please take another look at available information sources.
https://lore.kernel.org/cocci/f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00017.html

Another command example:
Markus_Elfring@Sonne:…/Projekte/Linux/next-patched> spatch --timeout 23 -j4 --chunksize 1 -dir . …/Projekte/Coccinelle/janitor/show_jumps_to_pointer_check.cocci > ../show_jumps_to_pointer_check-next-20230315.diff 2> ../show_jumps_to_pointer_check-errors-next-20230315.txt


Regards,
Markus

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

* Re: [cocci] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()
  2023-03-17 14:20     ` [cocci] " Markus Elfring
@ 2023-03-17 15:41       ` Nathan Lynch
  2023-03-18  7:30         ` Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Nathan Lynch @ 2023-03-17 15:41 UTC (permalink / raw)
  To: Markus Elfring
  Cc: cocci, LKML, Christophe Leroy, Michael Ellerman, Nicholas Piggin,
	Paul Moore, linuxppc-dev, kernel-janitors

Markus Elfring <Markus.Elfring@web.de> writes:
>>> The label “out_err” was used to jump to another pointer check despite of
>>> the detail in the implementation of the function “pSeries_reconfig_add_node”
>>> that it was determined already that the corresponding variable contained
>>> a null pointer (because of a failed function call in two cases).
>>>
>>> 1. Thus return directly after a call of the function “kzalloc” failed.
>>>
>>> 2. Use more appropriate labels instead.
>>>
>>> 3. Delete a redundant check.
>>>
>>> 4. Omit an explicit initialisation for the local variable “err”.
>>>
>>> This issue was detected by using the Coccinelle software.
>> Is there a correctness or safety issue here?
>
> I got the impression that the application of only a single label like “out_err”
> resulted in improvable implementation details.

I don't understand what you're trying to say here. It doesn't seem to
answer my question.

>> The subject uses the word "fix" but the commit message doesn't seem to identify one.
>
> Can you find the proposed adjustments reasonable?

In the absence of a bug fix or an improvement in readability, not
really, sorry. It adds to the function more goto labels and another
return, apparently to avoid checks that are sometimes redundant (but not
incorrect) at the C source code level. An optimizing compiler doesn't
necessarily arrange the generated code in the same way.

>> Can you share how Coccinelle is being invoked and its output?
>
> Please take another look at available information sources.
> https://lore.kernel.org/cocci/f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de/

I wasn't cc'd on this and I'm not subscribed to any lists in the
recipients for that message, so not sure how I would take "another"
look. :-)

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

* Re: [cocci] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()
  2023-03-17 15:41       ` Nathan Lynch
@ 2023-03-18  7:30         ` Markus Elfring
  2023-03-20 15:38           ` Nathan Lynch
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-18  7:30 UTC (permalink / raw)
  To: Nathan Lynch, linuxppc-dev, kernel-janitors
  Cc: cocci, LKML, Christophe Leroy, Michael Ellerman, Nicholas Piggin,
	Paul Moore

>>>> The label “out_err” was used to jump to another pointer check despite of
>>>> the detail in the implementation of the function “pSeries_reconfig_add_node”
>>>> that it was determined already that the corresponding variable contained
>>>> a null pointer (because of a failed function call in two cases).
>>>>
>>>> 1. Thus return directly after a call of the function “kzalloc” failed.
>>>>
>>>> 2. Use more appropriate labels instead.
>>>>
>>>> 3. Delete a redundant check.
>>>>
>>>> 4. Omit an explicit initialisation for the local variable “err”.
>>>>
>>>> This issue was detected by using the Coccinelle software.
>>> Is there a correctness or safety issue here?
>> I got the impression that the application of only a single label like “out_err”
>> resulted in improvable implementation details.
> I don't understand what you're trying to say here.

What does hinder you to understand the presented change description better
at the moment?


> It doesn't seem to answer my question.


I hope that my answer will trigger further helpful considerations.


>>> The subject uses the word "fix" but the commit message doesn't seem to identify one.
>> Can you find the proposed adjustments reasonable?
> In the absence of a bug fix or an improvement in readability, not really, sorry.

The views are varying for “programming bugs”, aren't they?


> It adds to the function more goto labels and another return,

This is the suggested source code transformation.


> apparently to avoid checks

Can the support grow for such a programming goal?



> that are sometimes redundant

Can such implementation details become undesirable?


> (but not incorrect) at the C source code level.

Will this aspect affect further development concerns?



>> Please take another look at available information sources.
>> https://lore.kernel.org/cocci/f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de/
> I wasn't cc'd on this and I'm not subscribed to any lists in the recipients
> for that message, so not sure how I would take "another" look. :-)

I imagine that you can benefit more from information which can be retrieved
by archive interfaces also according to the mailing list of the Coccinelle software.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/dev-tools/coccinelle.rst?h=v6.3-rc2#n9

Regards,
Markus

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

* Re: [cocci] [PATCH] RDMA/siw: Fix exception handling in siw_accept_newconn()
       [not found] ` <afe30fc6-04c9-528c-f84a-67902b5a6ed8@web.de>
@ 2023-03-19 11:40   ` Leon Romanovsky
       [not found]     ` <1c06e86d-1468-c11a-8344-9563ad6047b5@web.de>
  0 siblings, 1 reply; 99+ messages in thread
From: Leon Romanovsky @ 2023-03-19 11:40 UTC (permalink / raw)
  To: Markus Elfring
  Cc: kernel-janitors, linux-rdma, Bernard Metzler, Jason Gunthorpe,
	cocci, LKML

On Sat, Mar 18, 2023 at 08:43:04PM +0100, Markus Elfring wrote:
> Date: Sat, 18 Mar 2023 20:30:12 +0100
> 
> The label “error” was used to jump to another pointer check despite of
> the detail in the implementation of the function “siw_accept_newconn”
> that it was determined already that corresponding variables contained
> still null pointers.
> 
> 1. Use more appropriate labels instead.
> 
> 2. Delete two questionable checks.
> 
> 3. Omit extra initialisations (for the variables “new_cep” and “new_s”)
>    which became unnecessary with this refactoring.
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 6c52fdc244b5ccc468006fd65a504d4ee33743c7 ("rdma/siw: connection management")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/sw/siw/siw_cm.c | 32 ++++++++++++++----------------
>  1 file changed, 15 insertions(+), 17 deletions(-)

Please read Documentation/process/submitting-patches.rst and resubmit.
Your patch is not valid.

Thanks

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

* Re: [cocci] [PATCH] RDMA/erdma: Fix exception handling in erdma_accept_newconn()
       [not found] ` <f0f96f74-21d1-f5bf-1086-1c3ce0ea18f5@web.de>
@ 2023-03-19 11:41   ` Leon Romanovsky
  2023-03-19 13:36   ` Cheng Xu
  1 sibling, 0 replies; 99+ messages in thread
From: Leon Romanovsky @ 2023-03-19 11:41 UTC (permalink / raw)
  To: Markus Elfring
  Cc: kernel-janitors, linux-rdma, Cheng Xu, Jason Gunthorpe, Kai Shen,
	Yang Li, cocci, LKML

On Sat, Mar 18, 2023 at 09:15:58PM +0100, Markus Elfring wrote:
> Date: Sat, 18 Mar 2023 21:08:58 +0100
> 
> The label “error” was used to jump to another pointer check despite of
> the detail in the implementation of the function “erdma_accept_newconn”
> that it was determined already that corresponding variables contained
> still null pointers.
> 
> 1. Thus return directly if
>    * the cep state is not the value “ERDMA_EPSTATE_LISTENING”
>      or
>    * a call of the function “erdma_cep_alloc” failed.
> 
> 2. Use more appropriate labels instead.
> 
> 3. Delete two questionable checks.
> 
> 4. Omit extra initialisations (for the variables “new_cep”, “new_s” and “ret”)
>    which became unnecessary with this refactoring.
> 
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 920d93eac8b97778fef48f34f10e58ddf870fc2a ("RDMA/erdma: Add connection management (CM) support")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/erdma/erdma_cm.c | 39 +++++++++++---------------
>  1 file changed, 17 insertions(+), 22 deletions(-)

Same comment as for your RDMA/siw patch.

Thanks

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

* Re: [cocci] [PATCH] RDMA/erdma: Fix exception handling in erdma_accept_newconn()
       [not found] ` <f0f96f74-21d1-f5bf-1086-1c3ce0ea18f5@web.de>
  2023-03-19 11:41   ` [cocci] [PATCH] RDMA/erdma: Fix exception handling in erdma_accept_newconn() Leon Romanovsky
@ 2023-03-19 13:36   ` Cheng Xu
  2023-03-19 14:00     ` Markus Elfring
  1 sibling, 1 reply; 99+ messages in thread
From: Cheng Xu @ 2023-03-19 13:36 UTC (permalink / raw)
  To: Markus Elfring, kernel-janitors, linux-rdma, Jason Gunthorpe,
	Kai Shen, Leon Romanovsky, Yang Li
  Cc: cocci, LKML



On 3/19/23 4:15 AM, Markus Elfring wrote:
> Date: Sat, 18 Mar 2023 21:08:58 +0100
> 

<...>

> +disassoc_socket:
> +    erdma_socket_disassoc(new_s);
> +    sock_release(new_s);
> +    new_cep->state = ERDMA_EPSTATE_CLOSED;
> +    erdma_cancel_mpatimer(new_cep);
> +put_cep:
> +    erdma_cep_put(new_cep);> +    new_cep->sock = NULL;

Thanks, but this causes an use-after-free issue because new_cep will be
released after last erdma_cep_put being called.

Cheng Xu

>  }
>  
>  static int erdma_newconn_connected(struct erdma_cep *cep)

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

* Re: [cocci] [PATCH] RDMA/erdma: Fix exception handling in erdma_accept_newconn()
  2023-03-19 13:36   ` Cheng Xu
@ 2023-03-19 14:00     ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-19 14:00 UTC (permalink / raw)
  To: Cheng Xu, kernel-janitors, linux-rdma, Jason Gunthorpe, Kai Shen,
	Leon Romanovsky, Yang Li
  Cc: cocci, LKML

>> +disassoc_socket:
>> +    erdma_socket_disassoc(new_s);
>> +    sock_release(new_s);
>> +    new_cep->state = ERDMA_EPSTATE_CLOSED;
>> +    erdma_cancel_mpatimer(new_cep);
>> +put_cep:
>> +    erdma_cep_put(new_cep);> +    new_cep->sock = NULL;
> Thanks, but this causes an use-after-free issue because new_cep will be
> released after last erdma_cep_put being called.


How do you think about to omit the statement “new_cep->sock = NULL;” then
which I preserved (in my change suggestion) according to the currently active source code)?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/infiniband/hw/erdma/erdma_cm.c?h=v6.3-rc2#n702

Regards,
Markus


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

* Re: [cocci] [PATCH] RDMA/siw: Fix exception handling in siw_accept_newconn()
       [not found]     ` <1c06e86d-1468-c11a-8344-9563ad6047b5@web.de>
@ 2023-03-19 14:11       ` Leon Romanovsky
       [not found]         ` <a03c1d04-a41e-7722-c36a-bd6f61094702@web.de>
  0 siblings, 1 reply; 99+ messages in thread
From: Leon Romanovsky @ 2023-03-19 14:11 UTC (permalink / raw)
  To: Markus Elfring
  Cc: kernel-janitors, linux-rdma, Bernard Metzler, Jason Gunthorpe,
	cocci, LKML

On Sun, Mar 19, 2023 at 02:38:03PM +0100, Markus Elfring wrote:
> >> Date: Sat, 18 Mar 2023 20:30:12 +0100
> >>
> >> The label “error” was used to jump to another pointer check despite of
> >> the detail in the implementation of the function “siw_accept_newconn”
> >> that it was determined already that corresponding variables contained
> >> still null pointers.
> >>
> >> 1. Use more appropriate labels instead.
> >>
> >> 2. Delete two questionable checks.
> >>
> >> 3. Omit extra initialisations (for the variables “new_cep” and “new_s”)
> >>    which became unnecessary with this refactoring.
> >>
> >> This issue was detected by using the Coccinelle software.
> >>
> >> Fixes: 6c52fdc244b5ccc468006fd65a504d4ee33743c7 ("rdma/siw: connection management")
> >> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> >> ---
> >>  drivers/infiniband/sw/siw/siw_cm.c | 32 ++++++++++++++----------------
> >>  1 file changed, 15 insertions(+), 17 deletions(-)
> > Please read Documentation/process/submitting-patches.rst and resubmit.
> > Your patch is not valid.
> 
> 
> What do you find improvable here?

Did you read the guide above?

1. The patch is malformed and doesn't appear in lore and patchworks.
2. "Date ..." in the middle of patch
3. Wrong Fixes line.
4. Patch contains too much and too different things at the same time.

Thanks

> 
> Regards,
> Markus
> 

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

* [cocci] [PATCH 0/2] irqchip/gic-v4: Adjustments for two function implementations
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (3 preceding siblings ...)
       [not found] ` <f0f96f74-21d1-f5bf-1086-1c3ce0ea18f5@web.de>
@ 2023-03-19 20:00 ` Markus Elfring
  2023-03-25 10:25   ` [cocci] [PATCH resent " Markus Elfring
       [not found] ` <521b63e1-9470-58ef-599e-50a1846e5380@web.de>
                   ` (25 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-19 20:00 UTC (permalink / raw)
  To: kernel-janitors, Marc Zyngier, Thomas Gleixner, Zenghui Yu; +Cc: cocci, LKML

Date: Sun, 19 Mar 2023 20:54:05 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Fix exception handling in its_alloc_vcpu_irqs()
  Fix exception handling in its_alloc_vcpu_sgis()

 drivers/irqchip/irq-gic-v4.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

--
2.40.0



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

* Re: [cocci] RDMA/siw: Fix exception handling in siw_accept_newconn()
       [not found]           ` <20230319173716.GF36557@unreal>
@ 2023-03-20  8:19             ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-20  8:19 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: kernel-janitors, linux-rdma, Bernard Metzler, Jason Gunthorpe,
	cocci, LKML, Cheng Xu

> ok, I'm taking my request to resubmit back.


Interesting …



> Please retain from posting to RDMA ML. I'm not going to apply any of
> your patches.


This is a pity.

It seems that I need to convince other contributors somehow
to care more for remaining programming mistakes.


Example:
Use after free (or erdma_cep_put)?

https://lore.kernel.org/linux-rdma/167179d0-e1ea-39a8-4143-949ad57294c2@linux.alibaba.com/
https://lkml.org/lkml/2023/3/19/191
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00026.html


Regards,
Markus


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

* Re: [cocci] [PATCH] Input: iforce - Fix exception handling in iforce_usb_probe()
       [not found]   ` <ZBffPEIWcmYcaXR3@google.com>
@ 2023-03-20  8:55     ` Markus Elfring
       [not found]     ` <6b5de584-b31f-9045-a438-b42da350326b@I-love.SAKURA.ne.jp>
  1 sibling, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-20  8:55 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: kernel-janitors, linux-input, Hillf Danton, Tetsuo Handa, cocci, LKML

>> Date: Sun, 19 Mar 2023 18:50:51 +0100
>>
>> The label “fail” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “iforce_usb_probe”
>> that it was determined already that a corresponding variable contained
>> still a null pointer.
>>
>> 1. Use more appropriate labels instead.
>>
>> 2. Reorder jump targets at the end.
>>
>> 3. Delete a redundant check.
>>
>>
>> This issue was detected by using the Coccinelle software.
> I am sorry, but I do not understand what the actual issue is.


* Would you find a duplicate pointer check questionable?

* Will you become into the mood to adjust the usage of a single label like “fail”?


> The fact that come Coccinelle script complains is not enough to change the code.


Can you understand the corresponding clarification request better?


Reconsidering repeated pointer checks with SmPL

https://lore.kernel.org/cocci/f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00017.html



Regards,
Markus

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

* Re: [cocci] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()
  2023-03-18  7:30         ` Markus Elfring
@ 2023-03-20 15:38           ` Nathan Lynch
  2023-03-21  6:54             ` Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Nathan Lynch @ 2023-03-20 15:38 UTC (permalink / raw)
  To: Markus Elfring
  Cc: cocci, LKML, Christophe Leroy, Michael Ellerman, Nicholas Piggin,
	Paul Moore, linuxppc-dev, kernel-janitors

Markus Elfring <Markus.Elfring@web.de> writes:
>>>>> The label “out_err” was used to jump to another pointer check despite of
>>>>> the detail in the implementation of the function “pSeries_reconfig_add_node”
>>>>> that it was determined already that the corresponding variable contained
>>>>> a null pointer (because of a failed function call in two cases).
>>>>>
>>>>> 1. Thus return directly after a call of the function “kzalloc” failed.
>>>>>
>>>>> 2. Use more appropriate labels instead.
>>>>>
>>>>> 3. Delete a redundant check.
>>>>>
>>>>> 4. Omit an explicit initialisation for the local variable “err”.
>>>>>
>>>>> This issue was detected by using the Coccinelle software.
>>>> Is there a correctness or safety issue here?
>>> I got the impression that the application of only a single label like “out_err”
>>> resulted in improvable implementation details.
>> I don't understand what you're trying to say here.
>
> What does hinder you to understand the presented change description better
> at the moment?
>
>
>> It doesn't seem to answer my question.
>
>
> I hope that my answer will trigger further helpful considerations.

I don't consider this response constructive, but I want to get this back
on track. It's been brought to my attention that there is in fact a
crash bug in this function's error path:

	np->parent = pseries_of_derive_parent(path);
	if (IS_ERR(np->parent)) {
		err = PTR_ERR(np->parent);
		goto out_err;
	}
...
out_err:
	if (np) {
		of_node_put(np->parent);

np->parent can be an encoded error value, we don't want to of_node_put()
that.

I believe the patch as written happens to fix the issue. Will you please
write it up as a bug fix and resubmit?

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

* [cocci] [PATCH 0/2] md/raid: Adjustments for two function implementations
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (5 preceding siblings ...)
       [not found] ` <521b63e1-9470-58ef-599e-50a1846e5380@web.de>
@ 2023-03-20 16:47 ` Markus Elfring
  2023-03-25  9:20   ` [cocci] [PATCH resent " Markus Elfring
  2023-03-21 16:07 ` [cocci] [PATCH 0/3] scsi: message: fusion: Adjustments for four " Markus Elfring
                   ` (23 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-20 16:47 UTC (permalink / raw)
  To: kernel-janitors, linux-raid, Coly Li, Jens Axboe,
	Kent Overstreet, Maciej Trela, Neil Brown, Shaohua Li, Song Liu
  Cc: cocci, LKML

Date: Mon, 20 Mar 2023 17:28:05 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  raid1: Fix exception handling in setup_conf()
  raid10: Fix exception handling in setup_conf()

 drivers/md/raid1.c  | 75 ++++++++++++++++++++++++++-------------------
 drivers/md/raid10.c | 42 +++++++++++++------------
 2 files changed, 67 insertions(+), 50 deletions(-)

--
2.40.0



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

* Re: [cocci] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()
  2023-03-20 15:38           ` Nathan Lynch
@ 2023-03-21  6:54             ` Markus Elfring
  2023-03-21 10:30               ` [cocci] [PATCH v2 0/2] powerpc/pseries: Fixes for " Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-21  6:54 UTC (permalink / raw)
  To: Nathan Lynch
  Cc: cocci, LKML, Christophe Leroy, Michael Ellerman, Nicholas Piggin,
	Paul Moore, linuxppc-dev, kernel-janitors

> It's been brought to my attention that there is in fact a crash bug
> in this function's error path:

How do you think about to mention any other contributors for attribution
according to this issue?


> np->parent can be an encoded error value, we don't want to of_node_put() that.

Will the development attention grow for any more cases?


> I believe the patch as written happens to fix the issue.

Is it interesting how many details can still be improved (by my change suggestion)
also for the discussed function implementation?


> Will you please write it up as a bug fix and resubmit?

Another proposal will follow.

Regards,
Markus

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

* [cocci] [PATCH v2 0/2] powerpc/pseries: Fixes for exception handling in pSeries_reconfig_add_node()
  2023-03-21  6:54             ` Markus Elfring
@ 2023-03-21 10:30               ` Markus Elfring
  2023-03-25 13:40                 ` [cocci] [PATCH resent " Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-21 10:30 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Christophe Leroy,
	Michael Ellerman, Nathan Lynch, Nicholas Piggin, Paul Moore
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 11:26:32 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Do not pass an error pointer to of_node_put()
  Fix exception handling

 arch/powerpc/platforms/pseries/reconfig.c | 26 ++++++++++++-----------
 1 file changed, 14 insertions(+), 12 deletions(-)

--
2.40.0



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

* [cocci] [PATCH 0/3] scsi: message: fusion: Adjustments for four function implementations
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (6 preceding siblings ...)
  2023-03-20 16:47 ` [cocci] [PATCH 0/2] md/raid: Adjustments for two function implementations Markus Elfring
@ 2023-03-21 16:07 ` Markus Elfring
  2023-03-25  9:00   ` [cocci] [PATCH resent " Markus Elfring
       [not found] ` <e3aaeecf-8e74-2e74-c58a-d80e153e98f9@web.de>
                   ` (22 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-21 16:07 UTC (permalink / raw)
  To: kernel-janitors, linux-scsi, MPT-FusionLinux.pdl, Sathya Prakash,
	Sreekanth Reddy, Suganath Prabu Subramani
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 16:16:44 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Return directly after input data validation failed
  Delete a redundant pointer check
  Delete an unnecessary variable initialisation

 drivers/message/fusion/mptbase.c | 49 +++++++++-----------------
 drivers/message/fusion/mptsas.c  | 59 ++++++++++++--------------------
 2 files changed, 38 insertions(+), 70 deletions(-)

--
2.40.0



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

* Re: [cocci] [PATCH] media: hantro: HEVC: Fix exception handling in tile_buffer_reallocate()
       [not found] ` <e3aaeecf-8e74-2e74-c58a-d80e153e98f9@web.de>
@ 2023-03-22  9:36   ` Benjamin Gaignard
  2023-03-22 14:45     ` Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Benjamin Gaignard @ 2023-03-22  9:36 UTC (permalink / raw)
  To: Markus Elfring, kernel-janitors, linux-media, linux-rockchip,
	Adrian Ratiu, Ezequiel Garcia, Hans Verkuil,
	Mauro Carvalho Chehab, Philipp Zabel
  Cc: cocci, LKML

Hi Markus,

Thanks for your patch,

Le 20/03/2023 à 19:43, Markus Elfring a écrit :
> Date: Mon, 20 Mar 2023 19:13:20 +0100
>
> The label “err_free_tile_buffers” was used to jump to another pointer
> check despite of the detail in the implementation of the function
> “tile_buffer_reallocate” that it was determined already that
> a corresponding variable contained a null pointer because of a failed
> function call “dma_alloc_coherent”.
>
> * Thus use an additional label instead.
>
> * Delete a redundant check.
>
>
> This issue was detected by using the Coccinelle software.

If you want to optimize the error path I think the best
option is to return -ENOMEM when hevc_dec->tile_filter.cpu is NULL,
remove
	if (hevc_dec->tile_bsd.cpu)
		dma_free_coherent(vpu->dev, hevc_dec->tile_bsd.size,
				  hevc_dec->tile_bsd.cpu,
				  hevc_dec->tile_bsd.dma);
and reorder the two other dma_free to get something clean.

Regards,
Benjamin

>
> Fixes: cb5dd5a0fa518dff14ff2b90837c3c8f98f4dd5c ("media: hantro: Introduce G2/HEVC decoder")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>   drivers/media/platform/verisilicon/hantro_hevc.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/verisilicon/hantro_hevc.c b/drivers/media/platform/verisilicon/hantro_hevc.c
> index 9383fb7081f6..ac60df18efb7 100644
> --- a/drivers/media/platform/verisilicon/hantro_hevc.c
> +++ b/drivers/media/platform/verisilicon/hantro_hevc.c
> @@ -109,7 +109,7 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
>                                  &hevc_dec->tile_filter.dma,
>                                  GFP_KERNEL);
>       if (!hevc_dec->tile_filter.cpu)
> -        goto err_free_tile_buffers;
> +        goto recheck_tile_sao_cpu;
>       hevc_dec->tile_filter.size = size;
>   
>       size = (VERT_SAO_RAM_SIZE * height64 * (num_tile_cols - 1) * ctx->bit_depth) / 8;
> @@ -133,12 +133,12 @@ static int tile_buffer_reallocate(struct hantro_ctx *ctx)
>       return 0;
>   
>   err_free_tile_buffers:
> -    if (hevc_dec->tile_filter.cpu)
> -        dma_free_coherent(vpu->dev, hevc_dec->tile_filter.size,
> -                  hevc_dec->tile_filter.cpu,
> -                  hevc_dec->tile_filter.dma);
> +    dma_free_coherent(vpu->dev, hevc_dec->tile_filter.size,
> +              hevc_dec->tile_filter.cpu,
> +              hevc_dec->tile_filter.dma);
>       hevc_dec->tile_filter.cpu = NULL;
>   
> +recheck_tile_sao_cpu:
>       if (hevc_dec->tile_sao.cpu)
>           dma_free_coherent(vpu->dev, hevc_dec->tile_sao.size,
>                     hevc_dec->tile_sao.cpu,
> --
> 2.40.0
>
>

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

* Re: [cocci] [PATCH] media: hantro: HEVC: Fix exception handling in tile_buffer_reallocate()
  2023-03-22  9:36   ` [cocci] [PATCH] media: hantro: HEVC: Fix exception handling in tile_buffer_reallocate() Benjamin Gaignard
@ 2023-03-22 14:45     ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-22 14:45 UTC (permalink / raw)
  To: Benjamin Gaignard, kernel-janitors, linux-media, linux-rockchip,
	Adrian Ratiu, Ezequiel Garcia, Hans Verkuil,
	Mauro Carvalho Chehab, Philipp Zabel
  Cc: cocci, LKML


>> The label “err_free_tile_buffers” was used to jump to another pointer
>> check despite of the detail in the implementation of the function
>> “tile_buffer_reallocate” that it was determined already that
>> a corresponding variable contained a null pointer because of a failed
>> function call “dma_alloc_coherent”.
>>
>> * Thus use an additional label instead.
>>
>> * Delete a redundant check.
>>
>>
>> This issue was detected by using the Coccinelle software.
>
> If you want to optimize the error path

I would find it nice if more contributors can pick such an idea up.


> I think the best option is to return -ENOMEM when hevc_dec->tile_filter.cpu is NULL,
> remove
>     if (hevc_dec->tile_bsd.cpu)
>         dma_free_coherent(vpu->dev, hevc_dec->tile_bsd.size,
>                   hevc_dec->tile_bsd.cpu,
>                   hevc_dec->tile_bsd.dma);
> and reorder the two other dma_free to get something clean.

It seems that my understanding was too limited so far for possible improvements
of this function implementation.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/verisilicon/hantro_hevc.c?id=a1effab7a3a35a837dd9d2b974a1bc4939df1ad5#n70

Would you like to apply any other design variant?

Regards,
Markus

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

* [cocci] [PATCH] btrfs: Fix exception handling in relocating_repair_kthread()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (8 preceding siblings ...)
       [not found] ` <e3aaeecf-8e74-2e74-c58a-d80e153e98f9@web.de>
@ 2023-03-22 19:20 ` Markus Elfring
  2023-03-25  8:25   ` [cocci] [PATCH resent] " Markus Elfring
  2023-03-22 21:20 ` [cocci] [PATCH] ufs: Fix exception handling in ufs_fill_super() Markus Elfring
                   ` (20 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-22 19:20 UTC (permalink / raw)
  To: kernel-janitors, linux-btrfs, Chris Mason, David Sterba,
	Josef Bacik, Naohiro Aota
  Cc: cocci, LKML

Date: Wed, 22 Mar 2023 20:10:09 +0100

The label “out” was used to jump to another pointer check despite of
the detail in the implementation of the function
“relocating_repair_kthread” that it was determined already that
a corresponding variable contained a null pointer because of
a failed call of the function “btrfs_lookup_block_group”.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: f7ef5287a63d644e62a52893af8c6cfcb5043213 ("btrfs: zoned: relocate block group to repair IO failure in zoned filesystems")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/btrfs/volumes.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6d0124b6e79e..de11ad6c8740 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -8096,23 +8096,22 @@ static int relocating_repair_kthread(void *data)
     /* Ensure block group still exists */
     cache = btrfs_lookup_block_group(fs_info, target);
     if (!cache)
-        goto out;
+        goto unlock;
 
     if (!test_bit(BLOCK_GROUP_FLAG_RELOCATING_REPAIR, &cache->runtime_flags))
-        goto out;
+        goto put_block_group;
 
     ret = btrfs_may_alloc_data_chunk(fs_info, target);
     if (ret < 0)
-        goto out;
+        goto put_block_group;
 
     btrfs_info(fs_info,
            "zoned: relocating block group %llu to repair IO failure",
            target);
     ret = btrfs_relocate_chunk(fs_info, target);
-
-out:
-    if (cache)
-        btrfs_put_block_group(cache);
+put_block_group:
+    btrfs_put_block_group(cache);
+unlock:
     mutex_unlock(&fs_info->reclaim_bgs_lock);
     btrfs_exclop_finish(fs_info);
     sb_end_write(fs_info->sb);
--
2.40.0



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

* [cocci] [PATCH] ufs: Fix exception handling in ufs_fill_super()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (9 preceding siblings ...)
  2023-03-22 19:20 ` [cocci] [PATCH] btrfs: Fix exception handling in relocating_repair_kthread() Markus Elfring
@ 2023-03-22 21:20 ` Markus Elfring
  2023-03-25  8:32   ` [cocci] [PATCH resent] " Markus Elfring
  2023-03-23 17:30 ` [cocci] [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace() Markus Elfring
                   ` (19 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-22 21:20 UTC (permalink / raw)
  To: kernel-janitors, Andrew Morton, Evgeniy Dushistov, Jesper Juhl
  Cc: cocci, LKML

Date: Wed, 22 Mar 2023 21:50:45 +0100

The label “failed” was used to jump to another pointer check despite of
the detail in the implementation of the function “ufs_fill_super”
that it was determined already that a corresponding variable contained
a null pointer because of a failed call of the function “kzalloc”
or “ubh_bread_uspi”.

1. Thus use two additional labels.

2. Delete a redundant check.

3. Omit extra assignments (for the variables “uspi” and “ubh”)
   at the beginning which became unnecessary with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: f99d49adf527fa6f7a9c42257fa76bca6b8df1e3 ("[PATCH] kfree cleanup: fs")
Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/ufs/super.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 23377c1baed9..017653c36080 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -789,8 +789,6 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
     unsigned maxsymlen;
     int ret = -EINVAL;
 
-    uspi = NULL;
-    ubh = NULL;
     flags = 0;
     
     UFSD("ENTER\n");
@@ -821,7 +819,7 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
     ufs_set_opt (sbi->s_mount_opt, ONERROR_LOCK);
     if (!ufs_parse_options ((char *) data, &sbi->s_mount_opt)) {
         pr_err("wrong mount options\n");
-        goto failed;
+        goto free_sbi;
     }
     if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
         if (!silent)
@@ -836,7 +834,7 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
     uspi = kzalloc(sizeof(struct ufs_sb_private_info), GFP_KERNEL);
     sbi->s_uspi = uspi;
     if (!uspi)
-        goto failed;
+        goto free_sbi;
     uspi->s_dirblksize = UFS_SECTOR_SIZE;
     super_block_offset=UFS_SBLOCK;
 
@@ -984,13 +982,13 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
     default:
         if (!silent)
             pr_err("unknown ufstype\n");
-        goto failed;
+        goto free_uspi;
     }
     
 again:    
     if (!sb_set_blocksize(sb, block_size)) {
         pr_err("failed to set blocksize\n");
-        goto failed;
+        goto free_uspi;
     }
 
     /*
@@ -1000,7 +998,7 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
     ubh = ubh_bread_uspi(uspi, sb, uspi->s_sbbase + super_block_offset/block_size, super_block_size);
     
     if (!ubh)
-            goto failed;
+            goto free_uspi;
 
     usb1 = ubh_get_usb_first(uspi);
     usb2 = ubh_get_usb_second(uspi);
@@ -1291,9 +1289,10 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
     return 0;
 
 failed:
-    if (ubh)
-        ubh_brelse_uspi (uspi);
-    kfree (uspi);
+    ubh_brelse_uspi(uspi);
+free_uspi:
+    kfree(uspi);
+free_sbi:
     kfree(sbi);
     sb->s_fs_info = NULL;
     UFSD("EXIT (FAILED)\n");
--
2.40.0



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

* [cocci] [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (10 preceding siblings ...)
  2023-03-22 21:20 ` [cocci] [PATCH] ufs: Fix exception handling in ufs_fill_super() Markus Elfring
@ 2023-03-23 17:30 ` Markus Elfring
  2023-03-24 17:30   ` Vlastimil Babka
  2023-03-23 21:12 ` [cocci] [PATCH] perf cputopo: Improve exception handling in build_cpu_topology() Markus Elfring
                   ` (18 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-23 17:30 UTC (permalink / raw)
  To: kernel-janitors, linux-mm, Andrew Morton, Kosaki Motohiro, Mel Gorman
  Cc: cocci, LKML

Date: Thu, 23 Mar 2023 18:18:59 +0100

The label “err_out” was used to jump to another pointer check despite of
the detail in the implementation of the function “shared_policy_replace”
that it was determined already that a corresponding variable contained a
null pointer because of a failed call of the function “kmem_cache_alloc”.

1. Use more appropriate labels instead.

2. The implementation of the function “mpol_put” contains a pointer check
   for its single input parameter.
   Thus delete a redundant check in the caller.


This issue was detected by using the Coccinelle software.

Fixes: 42288fe366c4f1ce7522bc9f27d0bc2a81c55264 ("mm: mempolicy: Convert shared_policy mutex to spinlock")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 mm/mempolicy.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index a256a241fd1d..fb0485688dcb 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2736,13 +2736,12 @@ static int shared_policy_replace(struct shared_policy *sp, unsigned long start,
         sp_insert(sp, new);
     write_unlock(&sp->lock);
     ret = 0;
+put_mpol:
+    mpol_put(mpol_new);
 
-err_out:
-    if (mpol_new)
-        mpol_put(mpol_new);
     if (n_new)
         kmem_cache_free(sn_cache, n_new);
-
+exit:
     return ret;
 
 alloc_new:
@@ -2750,10 +2749,10 @@ static int shared_policy_replace(struct shared_policy *sp, unsigned long start,
     ret = -ENOMEM;
     n_new = kmem_cache_alloc(sn_cache, GFP_KERNEL);
     if (!n_new)
-        goto err_out;
+        goto exit;
     mpol_new = kmem_cache_alloc(policy_cache, GFP_KERNEL);
     if (!mpol_new)
-        goto err_out;
+        goto put_mpol;
     atomic_set(&mpol_new->refcnt, 1);
     goto restart;
 }
--
2.40.0



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

* [cocci] [PATCH] perf cputopo: Improve exception handling in build_cpu_topology()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (11 preceding siblings ...)
  2023-03-23 17:30 ` [cocci] [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace() Markus Elfring
@ 2023-03-23 21:12 ` Markus Elfring
  2023-03-25  8:50   ` [cocci] [PATCH resent] " Markus Elfring
  2023-03-24 11:43 ` [cocci] [PATCH] perf pmu: Improve exception handling in pmu_lookup() Markus Elfring
                   ` (17 subsequent siblings)
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-23 21:12 UTC (permalink / raw)
  To: kernel-janitors, linux-perf-users, James Clark, Jiri Olsa,
	Leo Yan, Namhyung Kim, Suzuki Poulouse
  Cc: cocci, LKML, Alexander Shishkin, Peter Zijlstra

Date: Thu, 23 Mar 2023 22:00:07 +0100

The label “done” was used to jump to another pointer check despite of
the detail in the implementation of the function “build_cpu_topology”
that it was determined already that a corresponding variable contained
a null pointer because of a failed call of the function “fopen”.

1. Thus use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 5135d5efcbb439c2acb20d6197dd57af3a456e76 ("perf tools: Add cpu_topology object")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 tools/perf/util/cputopo.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/cputopo.c b/tools/perf/util/cputopo.c
index e08797c3cdbc..fd185951ee2c 100644
--- a/tools/perf/util/cputopo.c
+++ b/tools/perf/util/cputopo.c
@@ -112,10 +112,10 @@ static int build_cpu_topology(struct cpu_topology *tp, int cpu)
     }
     fp = fopen(filename, "r");
     if (!fp)
-        goto done;
+        goto exit;
 
     if (getline(&buf, &len, fp) <= 0)
-        goto done;
+        goto close_file;
 
     p = strchr(buf, '\n');
     if (p)
@@ -131,10 +131,10 @@ static int build_cpu_topology(struct cpu_topology *tp, int cpu)
         buf = NULL;
     }
     ret = 0;
-done:
-    if (fp)
-        fclose(fp);
     free(buf);
+close_file:
+    fclose(fp);
+exit:
     return ret;
 }
 
--
2.40.0



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

* [cocci] [PATCH] perf pmu: Improve exception handling in pmu_lookup()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (12 preceding siblings ...)
  2023-03-23 21:12 ` [cocci] [PATCH] perf cputopo: Improve exception handling in build_cpu_topology() Markus Elfring
@ 2023-03-24 11:43 ` Markus Elfring
  2023-03-24 14:12 ` [cocci] [PATCH] selftests/bpf: Improve exception handling in rbtree_add_and_remove() Markus Elfring
                   ` (16 subsequent siblings)
  30 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-24 11:43 UTC (permalink / raw)
  To: kernel-janitors, linux-perf-users, Adrian Hunter,
	Alexander Shishkin, Andi Kleen, Arnaldo Carvalho de Melo,
	Ian Rogers, Ingo Molnar, James Clark, Jin Yao, Jiri Olsa,
	Kan Liang, Mark Rutland, Namhyung Kim, Peter Zijlstra,
	Ravi Bangoria, Sean Christopherson, Suzuki Poulouse
  Cc: cocci, LKML

Date: Fri, 24 Mar 2023 12:15:24 +0100

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “pmu_lookup”
that it was determined already that a corresponding variable contained
a null pointer because of a failed call of the function “strdup”.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 13d60ba0738b0532edab3e1492b2005d36ba0802 ("perf pmu: Add PMU alias support")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 tools/perf/util/pmu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index c256b29defad..aafda41b6fe8 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -896,17 +896,17 @@ static struct perf_pmu *pmu_lookup(const char *lookup_name)
 	pmu->name = strdup(name);

 	if (!pmu->name)
-		goto err;
+		goto free_pmu;

 	/* Read type, and ensure that type value is successfully assigned (return 1) */
 	if (perf_pmu__scan_file(pmu, "type", "%u", &type) != 1)
-		goto err;
+		goto free_name;

 	alias_name = pmu_find_alias_name(name);
 	if (alias_name) {
 		pmu->alias_name = strdup(alias_name);
 		if (!pmu->alias_name)
-			goto err;
+			goto free_name;
 	}

 	pmu->type = type;
@@ -930,9 +930,9 @@ static struct perf_pmu *pmu_lookup(const char *lookup_name)
 	pmu->default_config = perf_pmu__get_default_config(pmu);

 	return pmu;
-err:
-	if (pmu->name)
-		free(pmu->name);
+free_name:
+	free(pmu->name);
+free_pmu:
 	free(pmu);
 	return NULL;
 }
--
2.40.0


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

* [cocci] [PATCH] selftests/bpf: Improve exception handling in rbtree_add_and_remove()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (13 preceding siblings ...)
  2023-03-24 11:43 ` [cocci] [PATCH] perf pmu: Improve exception handling in pmu_lookup() Markus Elfring
@ 2023-03-24 14:12 ` Markus Elfring
       [not found] ` <e6656c83-ee7a-a253-2028-109138779c94@web.de>
                   ` (15 subsequent siblings)
  30 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-24 14:12 UTC (permalink / raw)
  To: kernel-janitors, linux-kselftest, bpf, Alexei Starovoitov,
	Andrii Nakryiko, Daniel Borkmann, Dave Marchevsky, David Vernet,
	Hao Luo, Jiri Olsa, John Fastabend, KP Singh, Martin KaFai Lau,
	Mykola Lysenko, Shuah Khan, Song Liu, Stanislav Fomichev,
	Yonghong Song
  Cc: cocci, LKML

Date: Fri, 24 Mar 2023 14:54:18 +0100

The label “err_out” was used to jump to another pointer check despite of
the detail in the implementation of the function “rbtree_add_and_remove”
that it was determined already that a corresponding variable contained
a null pointer.

1. Thus return directly after the first call of the function
   “bpf_obj_new” failed.

2. Delete two questionable checks.

3. Omit an extra initialisation (for the variable “m”)
   which became unnecessary with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: 215249f6adc0359e3546829e7ee622b5e309b0ad ("selftests/bpf: Add rbtree selftests")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 tools/testing/selftests/bpf/progs/rbtree.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/rbtree.c b/tools/testing/selftests/bpf/progs/rbtree.c
index 4c90aa6abddd..9b6a65712498 100644
--- a/tools/testing/selftests/bpf/progs/rbtree.c
+++ b/tools/testing/selftests/bpf/progs/rbtree.c
@@ -75,16 +75,20 @@ SEC("tc")
 long rbtree_add_and_remove(void *ctx)
 {
 	struct bpf_rb_node *res = NULL;
-	struct node_data *n, *m = NULL;
+	struct node_data *n, *m;

 	n = bpf_obj_new(typeof(*n));
 	if (!n)
-		goto err_out;
+		return 1;
+
 	n->key = 5;

 	m = bpf_obj_new(typeof(*m));
-	if (!m)
-		goto err_out;
+	if (!m) {
+		bpf_obj_drop(n);
+		return 1;
+	}
+
 	m->key = 3;

 	bpf_spin_lock(&glock);
@@ -99,12 +103,6 @@ long rbtree_add_and_remove(void *ctx)
 	bpf_obj_drop(n);

 	return 0;
-err_out:
-	if (n)
-		bpf_obj_drop(n);
-	if (m)
-		bpf_obj_drop(m);
-	return 1;
 }

 SEC("tc")
--
2.40.0


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

* [cocci] [PATCH resent] drm/amd/display: Fix exception handling in dm_validate_stream_and_context()
       [not found] ` <e6656c83-ee7a-a253-2028-109138779c94@web.de>
@ 2023-03-24 15:42   ` Markus Elfring
       [not found]     ` <32674bac-92c2-8fc7-0977-6d2d81b3257f@amd.com>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-24 15:42 UTC (permalink / raw)
  To: kernel-janitors, amd-gfx, dri-devel, Alex Deucher,
	Aurabindo Pillai, Christian König, Daniel Vetter,
	David Airlie, Fangzhi Zuo, Harry Wentland, Hersen Wu, Leo Li,
	Rodrigo Siqueira, Roman Li, Stylon Wang, Xinhui Pan
  Cc: cocci, LKML

Date: Sat, 18 Mar 2023 16:21:32 +0100

The label “cleanup” was used to jump to another pointer check despite of
the detail in the implementation of the function “dm_validate_stream_and_context”
that it was determined already that corresponding variables contained
still null pointers.

1. Thus return directly if
   * a null pointer was passed for the function parameter “stream”
     or
   * a call of the function “dc_create_plane_state” failed.

2. Use a more appropriate label instead.

3. Delete two questionable checks.

4. Omit extra initialisations (for the variables “dc_state” and “dc_plane_state”)
   which became unnecessary with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: 5468c36d628524effbb89a9503eb1a2318804759 ("drm/amd/display: Filter Invalid 420 Modes for HDMI TMDS")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 ++++++++-----------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index eeaeca8b51f4..3086613f5f5d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6426,19 +6426,19 @@ static enum dc_status dm_validate_stream_and_context(struct dc *dc,
 						struct dc_stream_state *stream)
 {
 	enum dc_status dc_result = DC_ERROR_UNEXPECTED;
-	struct dc_plane_state *dc_plane_state = NULL;
-	struct dc_state *dc_state = NULL;
+	struct dc_plane_state *dc_plane_state;
+	struct dc_state *dc_state;

 	if (!stream)
-		goto cleanup;
+		return dc_result;

 	dc_plane_state = dc_create_plane_state(dc);
 	if (!dc_plane_state)
-		goto cleanup;
+		return dc_result;

 	dc_state = dc_create_state(dc);
 	if (!dc_state)
-		goto cleanup;
+		goto release_plane_state;

 	/* populate stream to plane */
 	dc_plane_state->src_rect.height  = stream->src.height;
@@ -6475,13 +6475,9 @@ static enum dc_status dm_validate_stream_and_context(struct dc *dc,
 	if (dc_result == DC_OK)
 		dc_result = dc_validate_global_state(dc, dc_state, true);

-cleanup:
-	if (dc_state)
-		dc_release_state(dc_state);
-
-	if (dc_plane_state)
-		dc_plane_state_release(dc_plane_state);
-
+	dc_release_state(dc_state);
+release_plane_state:
+	dc_plane_state_release(dc_plane_state);
 	return dc_result;
 }

--
2.40.0


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

* [cocci] [PATCH resent] ext4: Fix exception handling in parse_apply_sb_mount_options()
       [not found] ` <7214c986-4d0e-ad78-6312-c84515dc9abf@web.de>
@ 2023-03-24 17:02   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-24 17:02 UTC (permalink / raw)
  To: kernel-janitors, linux-ext4, Andreas Dilger, Carlos Maiolino,
	Eric Biggers, Lukas Czerner, Theodore Ts'o
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 22:33:06 +0100

The label “out_free” was used to jump to another pointer check
despite of the detail in the implementation of the function
“parse_apply_sb_mount_options” that it was determined already
that a corresponding variable contained a null pointer.

* Thus use an additional label.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 7edfd85b1ffd36593011dec96ab395912a340418 ("ext4: Completely separate options parsing and sb setup")
Fixes: c069db76ed7b681c69159f44be96d2137e9ca989 ("ext4: fix memory leak in parse_apply_sb_mount_options()")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/ext4/super.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e6d84c1e34a4..c0bc46956353 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2435,7 +2435,7 @@ static int parse_apply_sb_mount_options(struct super_block *sb,

 	fc = kzalloc(sizeof(struct fs_context), GFP_KERNEL);
 	if (!fc)
-		goto out_free;
+		goto free_mount_opts;

 	s_ctx = kzalloc(sizeof(struct ext4_fs_context), GFP_KERNEL);
 	if (!s_ctx)
@@ -2467,10 +2467,9 @@ static int parse_apply_sb_mount_options(struct super_block *sb,
 	ret = 0;

 out_free:
-	if (fc) {
-		ext4_fc_free(fc);
-		kfree(fc);
-	}
+	ext4_fc_free(fc);
+	kfree(fc);
+free_mount_opts:
 	kfree(s_mount_opts);
 	return ret;
 }
--
2.40.0


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

* Re: [cocci] [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace()
  2023-03-23 17:30 ` [cocci] [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace() Markus Elfring
@ 2023-03-24 17:30   ` Vlastimil Babka
  2023-03-24 18:03     ` Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Vlastimil Babka @ 2023-03-24 17:30 UTC (permalink / raw)
  To: Markus Elfring, kernel-janitors, linux-mm, Andrew Morton,
	Kosaki Motohiro, Mel Gorman
  Cc: cocci, LKML

Your patch doesn't apply, seems like it uses spaces instead of tabs. Also I
can't use 'b4' to download it as there are multiple different patches using
the same message-id:

https://lore.kernel.org/all/6e9ca062-939b-af96-c8ff-56ad485d6e79@web.de/

Re: subject, I don't see a bug that this would fix. You could say it's
"cleanup" and this function could use one, but for a cleanup it's not
improving the situation much.

On 3/23/23 18:30, Markus Elfring wrote:
> Date: Thu, 23 Mar 2023 18:18:59 +0100
> 
> The label “err_out” was used to jump to another pointer check despite of
> the detail in the implementation of the function “shared_policy_replace”
> that it was determined already that a corresponding variable contained a
> null pointer because of a failed call of the function “kmem_cache_alloc”.
> 
> 1. Use more appropriate labels instead.
> 
> 2. The implementation of the function “mpol_put” contains a pointer check
>    for its single input parameter.
>    Thus delete a redundant check in the caller.
> 
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 42288fe366c4f1ce7522bc9f27d0bc2a81c55264 ("mm: mempolicy: Convert shared_policy mutex to spinlock")

Again this is not a fix.

> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  mm/mempolicy.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index a256a241fd1d..fb0485688dcb 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2736,13 +2736,12 @@ static int shared_policy_replace(struct shared_policy *sp, unsigned long start,
>          sp_insert(sp, new);
>      write_unlock(&sp->lock);
>      ret = 0;
> +put_mpol:
> +    mpol_put(mpol_new);
>  
> -err_out:
> -    if (mpol_new)
> -        mpol_put(mpol_new);
>      if (n_new)
>          kmem_cache_free(sn_cache, n_new);
> -
> +exit:
>      return ret;
>  
>  alloc_new:
> @@ -2750,10 +2749,10 @@ static int shared_policy_replace(struct shared_policy *sp, unsigned long start,
>      ret = -ENOMEM;
>      n_new = kmem_cache_alloc(sn_cache, GFP_KERNEL);
>      if (!n_new)
> -        goto err_out;
> +        goto exit;

Just "return ret" and no need for exit label?

>      mpol_new = kmem_cache_alloc(policy_cache, GFP_KERNEL);
>      if (!mpol_new)
> -        goto err_out;
> +        goto put_mpol;

We are doing this because mpol_new == NULL, so we know there's no reason to
do mpol_put(), we could jump to the freeing of n_new.

>      atomic_set(&mpol_new->refcnt, 1);
>      goto restart;
>  }
> --
> 2.40.0
> 
> 
> 


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

* Re: [cocci] [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace()
  2023-03-24 17:30   ` Vlastimil Babka
@ 2023-03-24 18:03     ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-24 18:03 UTC (permalink / raw)
  To: Vlastimil Babka, kernel-janitors, linux-mm, Andrew Morton,
	Kosaki Motohiro, Mel Gorman
  Cc: cocci, LKML

> Your patch doesn't apply, seems like it uses spaces instead of tabs.

I am sorry for this glitch.

Andrew Morton picked my change suggestion up yesterday.
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-mempolicy-fix-exception-handling-in-shared_policy_replace.patch


> https://lore.kernel.org/all/6e9ca062-939b-af96-c8ff-56ad485d6e79@web.de/
>
> Re: subject, I don't see a bug that this would fix. You could say it's
> "cleanup" and this function could use one, but for a cleanup it's not
> improving the situation much.

I find a few details improvable also for the mentioned function implementation.


>> The label “err_out” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “shared_policy_replace”
>> that it was determined already that a corresponding variable contained a
>> null pointer because of a failed call of the function “kmem_cache_alloc”.
>>
>> 1. Use more appropriate labels instead.
>>
>> 2. The implementation of the function “mpol_put” contains a pointer check
>>    for its single input parameter.
>>    Thus delete a redundant check in the caller.
>>
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Fixes: 42288fe366c4f1ce7522bc9f27d0bc2a81c55264 ("mm: mempolicy: Convert shared_policy mutex to spinlock")
>
> Again this is not a fix.

Do you find the change description helpful?

Regards,
Markus

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

* Re: [cocci] [PATCH resent] drm/amd/display: Fix exception handling in dm_validate_stream_and_context()
       [not found]     ` <32674bac-92c2-8fc7-0977-6d2d81b3257f@amd.com>
@ 2023-03-24 18:19       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-24 18:19 UTC (permalink / raw)
  To: Hamza Mahfooz, kernel-janitors, amd-gfx, dri-devel, Alex Deucher,
	Aurabindo Pillai, Christian König, Daniel Vetter,
	David Airlie, Fangzhi Zuo, Harry Wentland, Hersen Wu, Leo Li,
	Rodrigo Siqueira, Roman Li, Stylon Wang, Xinhui Pan
  Cc: LKML, cocci

>> The label “cleanup” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “dm_validate_stream_and_context”
>> that it was determined already that corresponding variables contained
>> still null pointers.
>>
>> 1. Thus return directly if
>>    * a null pointer was passed for the function parameter “stream”
>>      or
>>    * a call of the function “dc_create_plane_state” failed.
>>
>> 2. Use a more appropriate label instead.
>>
>> 3. Delete two questionable checks.
>>
>> 4. Omit extra initialisations (for the variables “dc_state” and “dc_plane_state”)
>>    which became unnecessary with this refactoring.
>>
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Fixes: 5468c36d628524effbb89a9503eb1a2318804759 ("drm/amd/display: Filter Invalid 420 Modes for HDMI TMDS")
>
> Please truncate the hash to 12 characters.

May longer identifiers (or even the complete SHA-1 ID) occasionally also
be tolerated for the tag “Fixes”?

How do you think about the proposed change possibilities?

Regards,
Markus

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

* [cocci] [PATCH resent] btrfs: Fix exception handling in relocating_repair_kthread()
  2023-03-22 19:20 ` [cocci] [PATCH] btrfs: Fix exception handling in relocating_repair_kthread() Markus Elfring
@ 2023-03-25  8:25   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  8:25 UTC (permalink / raw)
  To: kernel-janitors, linux-btrfs, Chris Mason, David Sterba,
	Josef Bacik, Naohiro Aota
  Cc: cocci, LKML

Date: Wed, 22 Mar 2023 20:10:09 +0100

The label “out” was used to jump to another pointer check despite of
the detail in the implementation of the function
“relocating_repair_kthread” that it was determined already that
a corresponding variable contained a null pointer because of
a failed call of the function “btrfs_lookup_block_group”.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: f7ef5287a63d644e62a52893af8c6cfcb5043213 ("btrfs: zoned: relocate block group to repair IO failure in zoned filesystems")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/btrfs/volumes.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 6d0124b6e79e..de11ad6c8740 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -8096,23 +8096,22 @@ static int relocating_repair_kthread(void *data)
 	/* Ensure block group still exists */
 	cache = btrfs_lookup_block_group(fs_info, target);
 	if (!cache)
-		goto out;
+		goto unlock;

 	if (!test_bit(BLOCK_GROUP_FLAG_RELOCATING_REPAIR, &cache->runtime_flags))
-		goto out;
+		goto put_block_group;

 	ret = btrfs_may_alloc_data_chunk(fs_info, target);
 	if (ret < 0)
-		goto out;
+		goto put_block_group;

 	btrfs_info(fs_info,
 		   "zoned: relocating block group %llu to repair IO failure",
 		   target);
 	ret = btrfs_relocate_chunk(fs_info, target);
-
-out:
-	if (cache)
-		btrfs_put_block_group(cache);
+put_block_group:
+	btrfs_put_block_group(cache);
+unlock:
 	mutex_unlock(&fs_info->reclaim_bgs_lock);
 	btrfs_exclop_finish(fs_info);
 	sb_end_write(fs_info->sb);
--
2.40.0


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

* [cocci] [PATCH resent] ufs: Fix exception handling in ufs_fill_super()
  2023-03-22 21:20 ` [cocci] [PATCH] ufs: Fix exception handling in ufs_fill_super() Markus Elfring
@ 2023-03-25  8:32   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  8:32 UTC (permalink / raw)
  To: kernel-janitors, Andrew Morton, Evgeniy Dushistov; +Cc: cocci, LKML

Date: Wed, 22 Mar 2023 21:50:45 +0100

The label “failed” was used to jump to another pointer check despite of
the detail in the implementation of the function “ufs_fill_super”
that it was determined already that a corresponding variable contained
a null pointer because of a failed call of the function “kzalloc”
or “ubh_bread_uspi”.

1. Thus use two additional labels.

2. Delete a redundant check.

3. Omit extra assignments (for the variables “uspi” and “ubh”)
   at the beginning which became unnecessary with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: f99d49adf527fa6f7a9c42257fa76bca6b8df1e3 ("[PATCH] kfree cleanup: fs")
Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/ufs/super.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 23377c1baed9..017653c36080 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -789,8 +789,6 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 	unsigned maxsymlen;
 	int ret = -EINVAL;

-	uspi = NULL;
-	ubh = NULL;
 	flags = 0;

 	UFSD("ENTER\n");
@@ -821,7 +819,7 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 	ufs_set_opt (sbi->s_mount_opt, ONERROR_LOCK);
 	if (!ufs_parse_options ((char *) data, &sbi->s_mount_opt)) {
 		pr_err("wrong mount options\n");
-		goto failed;
+		goto free_sbi;
 	}
 	if (!(sbi->s_mount_opt & UFS_MOUNT_UFSTYPE)) {
 		if (!silent)
@@ -836,7 +834,7 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 	uspi = kzalloc(sizeof(struct ufs_sb_private_info), GFP_KERNEL);
 	sbi->s_uspi = uspi;
 	if (!uspi)
-		goto failed;
+		goto free_sbi;
 	uspi->s_dirblksize = UFS_SECTOR_SIZE;
 	super_block_offset=UFS_SBLOCK;

@@ -984,13 +982,13 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 	default:
 		if (!silent)
 			pr_err("unknown ufstype\n");
-		goto failed;
+		goto free_uspi;
 	}

 again:
 	if (!sb_set_blocksize(sb, block_size)) {
 		pr_err("failed to set blocksize\n");
-		goto failed;
+		goto free_uspi;
 	}

 	/*
@@ -1000,7 +998,7 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 	ubh = ubh_bread_uspi(uspi, sb, uspi->s_sbbase + super_block_offset/block_size, super_block_size);

 	if (!ubh)
-            goto failed;
+            goto free_uspi;

 	usb1 = ubh_get_usb_first(uspi);
 	usb2 = ubh_get_usb_second(uspi);
@@ -1291,9 +1289,10 @@ static int ufs_fill_super(struct super_block *sb, void *data, int silent)
 	return 0;

 failed:
-	if (ubh)
-		ubh_brelse_uspi (uspi);
-	kfree (uspi);
+	ubh_brelse_uspi(uspi);
+free_uspi:
+	kfree(uspi);
+free_sbi:
 	kfree(sbi);
 	sb->s_fs_info = NULL;
 	UFSD("EXIT (FAILED)\n");
--
2.40.0


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

* [cocci] [PATCH resent] perf cputopo: Improve exception handling in build_cpu_topology()
  2023-03-23 21:12 ` [cocci] [PATCH] perf cputopo: Improve exception handling in build_cpu_topology() Markus Elfring
@ 2023-03-25  8:50   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  8:50 UTC (permalink / raw)
  To: kernel-janitors, linux-perf-users, James Clark, Jiri Olsa,
	Leo Yan, Namhyung Kim, Suzuki Poulouse
  Cc: cocci, LKML, Alexander Shishkin, Peter Zijlstra

Date: Thu, 23 Mar 2023 22:00:07 +0100

The label “done” was used to jump to another pointer check despite of
the detail in the implementation of the function “build_cpu_topology”
that it was determined already that a corresponding variable contained
a null pointer because of a failed call of the function “fopen”.

1. Thus use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 5135d5efcbb439c2acb20d6197dd57af3a456e76 ("perf tools: Add cpu_topology object")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 tools/perf/util/cputopo.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/cputopo.c b/tools/perf/util/cputopo.c
index e08797c3cdbc..fd185951ee2c 100644
--- a/tools/perf/util/cputopo.c
+++ b/tools/perf/util/cputopo.c
@@ -112,10 +112,10 @@ static int build_cpu_topology(struct cpu_topology *tp, int cpu)
 	}
 	fp = fopen(filename, "r");
 	if (!fp)
-		goto done;
+		goto exit;

 	if (getline(&buf, &len, fp) <= 0)
-		goto done;
+		goto close_file;

 	p = strchr(buf, '\n');
 	if (p)
@@ -131,10 +131,10 @@ static int build_cpu_topology(struct cpu_topology *tp, int cpu)
 		buf = NULL;
 	}
 	ret = 0;
-done:
-	if (fp)
-		fclose(fp);
 	free(buf);
+close_file:
+	fclose(fp);
+exit:
 	return ret;
 }

--
2.40.0


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

* [cocci] [PATCH resent 0/3] scsi: message: fusion: Adjustments for four function implementations
  2023-03-21 16:07 ` [cocci] [PATCH 0/3] scsi: message: fusion: Adjustments for four " Markus Elfring
@ 2023-03-25  9:00   ` Markus Elfring
  2023-03-25  9:03     ` [cocci] [PATCH resent 1/3] scsi: message: fusion: Return directly after input data validation failed in four functions Markus Elfring
                       ` (2 more replies)
  0 siblings, 3 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:00 UTC (permalink / raw)
  To: kernel-janitors, linux-scsi, MPT-FusionLinux.pdl, Sathya Prakash,
	Sreekanth Reddy, Suganath Prabu Subramani
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 16:16:44 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Return directly after input data validation failed
  Delete a redundant pointer check
  Delete an unnecessary variable initialisation

 drivers/message/fusion/mptbase.c | 49 +++++++++-----------------
 drivers/message/fusion/mptsas.c  | 59 ++++++++++++--------------------
 2 files changed, 38 insertions(+), 70 deletions(-)

--
2.40.0


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

* [cocci] [PATCH resent 1/3] scsi: message: fusion: Return directly after input data validation failed in four functions
  2023-03-25  9:00   ` [cocci] [PATCH resent " Markus Elfring
@ 2023-03-25  9:03     ` Markus Elfring
  2023-03-25  9:07     ` [cocci] [PATCH resent 2/3] scsi: message: fusion: Delete a redundant pointer check " Markus Elfring
  2023-03-25  9:10     ` [cocci] [PATCH resent 3/3] scsi: message: fusion: Delete an unnecessary variable initialisation " Markus Elfring
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:03 UTC (permalink / raw)
  To: kernel-janitors, linux-scsi, MPT-FusionLinux.pdl, Sathya Prakash,
	Sreekanth Reddy, Suganath Prabu Subramani
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 15:02:37 +0100

Return directly after input data validation failed at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/message/fusion/mptbase.c | 18 ++++++------------
 drivers/message/fusion/mptsas.c  | 18 ++++++------------
 2 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 9b3ba2df71c7..4b097c1f95cb 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -5665,17 +5665,14 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 	cfg.cfghdr.hdr = &hdr;
 	cfg.action = MPI_CONFIG_ACTION_PAGE_HEADER;

-	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
-
-	if (!hdr.PageLength)
-		goto out;
+	if (mpt_config(ioc, &cfg) || !hdr.PageLength)
+		return;

 	buffer = dma_alloc_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
 				    &dma_handle, GFP_KERNEL);

 	if (!buffer)
-		goto out;
+		return;

 	cfg.physAddr = dma_handle;
 	cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;
@@ -6249,17 +6246,14 @@ mpt_get_manufacturing_pg_0(MPT_ADAPTER *ioc)
 	cfg.action = MPI_CONFIG_ACTION_PAGE_HEADER;
 	cfg.timeout = 10;

-	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
-
-	if (!cfg.cfghdr.hdr->PageLength)
-		goto out;
+	if (mpt_config(ioc, &cfg) || !cfg.cfghdr.hdr->PageLength)
+		return;

 	cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;
 	pbuf = dma_alloc_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
 				  &buf_dma, GFP_KERNEL);
 	if (!pbuf)
-		goto out;
+		return;

 	cfg.physAddr = buf_dma;

diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 88fe4a860ae5..e0e861e79e64 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -696,17 +696,14 @@ mptsas_add_device_component_starget_ir(MPT_ADAPTER *ioc,
 	cfg.action = MPI_CONFIG_ACTION_PAGE_HEADER;
 	cfg.timeout = SAS_CONFIG_PAGE_TIMEOUT;

-	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
-
-	if (!hdr.PageLength)
-		goto out;
+	if (mpt_config(ioc, &cfg) || !hdr.PageLength)
+		return;

 	buffer = dma_alloc_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
 				    &dma_handle, GFP_KERNEL);

 	if (!buffer)
-		goto out;
+		return;

 	cfg.physAddr = dma_handle;
 	cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;
@@ -4267,17 +4264,14 @@ mptsas_adding_inactive_raid_components(MPT_ADAPTER *ioc, u8 channel, u8 id)
 	cfg.action = MPI_CONFIG_ACTION_PAGE_HEADER;
 	cfg.timeout = SAS_CONFIG_PAGE_TIMEOUT;

-	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
-
-	if (!hdr.PageLength)
-		goto out;
+	if (mpt_config(ioc, &cfg) || !hdr.PageLength)
+		return;

 	buffer = dma_alloc_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
 				    &dma_handle, GFP_KERNEL);

 	if (!buffer)
-		goto out;
+		return;

 	cfg.physAddr = dma_handle;
 	cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;
--
2.40.0


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

* [cocci] [PATCH resent 2/3] scsi: message: fusion: Delete a redundant pointer check in four functions
  2023-03-25  9:00   ` [cocci] [PATCH resent " Markus Elfring
  2023-03-25  9:03     ` [cocci] [PATCH resent 1/3] scsi: message: fusion: Return directly after input data validation failed in four functions Markus Elfring
@ 2023-03-25  9:07     ` Markus Elfring
  2023-03-25  9:10     ` [cocci] [PATCH resent 3/3] scsi: message: fusion: Delete an unnecessary variable initialisation " Markus Elfring
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:07 UTC (permalink / raw)
  To: kernel-janitors, linux-scsi, MPT-FusionLinux.pdl, Sathya Prakash,
	Sreekanth Reddy, Suganath Prabu Subramani
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 16:00:25 +0100

* Use a more appropriate label instead.

* Remove an extra pointer check which became unnecessary.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/message/fusion/mptbase.c | 27 ++++++++---------------
 drivers/message/fusion/mptsas.c  | 37 +++++++++++---------------------
 2 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 4b097c1f95cb..59420590c5f7 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -5677,11 +5677,8 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 	cfg.physAddr = dma_handle;
 	cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;

-	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
-
-	if (!buffer->NumPhysDisks)
-		goto out;
+	if (mpt_config(ioc, &cfg) || !buffer->NumPhysDisks)
+		goto free_dma;

 	handle_inactive_volumes =
 	   (buffer->VolumeStatus.Flags & MPI_RAIDVOL0_STATUS_FLAG_VOLUME_INACTIVE ||
@@ -5690,7 +5687,7 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 	    buffer->VolumeStatus.State == MPI_RAIDVOL0_STATUS_STATE_MISSING) ? 1 : 0;

 	if (!handle_inactive_volumes)
-		goto out;
+		goto free_dma;

 	mutex_lock(&ioc->raid_data.inactive_list_mutex);
 	for (i = 0; i < buffer->NumPhysDisks; i++) {
@@ -5713,11 +5710,9 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 		    &ioc->raid_data.inactive_list);
 	}
 	mutex_unlock(&ioc->raid_data.inactive_list_mutex);
-
- out:
-	if (buffer)
-		dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
-				  buffer, dma_handle);
+free_dma:
+	dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
+			  buffer, dma_handle);
 }

 /**
@@ -6258,17 +6253,13 @@ mpt_get_manufacturing_pg_0(MPT_ADAPTER *ioc)
 	cfg.physAddr = buf_dma;

 	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
+		goto free_dma;

 	memcpy(ioc->board_name, pbuf->BoardName, sizeof(ioc->board_name));
 	memcpy(ioc->board_assembly, pbuf->BoardAssembly, sizeof(ioc->board_assembly));
 	memcpy(ioc->board_tracer, pbuf->BoardTracerNumber, sizeof(ioc->board_tracer));
-
-out:
-
-	if (pbuf)
-		dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4, pbuf,
-				  buf_dma);
+free_dma:
+	dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4, pbuf, buf_dma);
 }

 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index e0e861e79e64..5c51339dfb9f 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -708,11 +708,8 @@ mptsas_add_device_component_starget_ir(MPT_ADAPTER *ioc,
 	cfg.physAddr = dma_handle;
 	cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;

-	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
-
-	if (!buffer->NumPhysDisks)
-		goto out;
+	if (mpt_config(ioc, &cfg) || !buffer->NumPhysDisks)
+		goto free_dma;

 	/*
 	 * Adding entry for hidden components
@@ -763,11 +760,9 @@ mptsas_add_device_component_starget_ir(MPT_ADAPTER *ioc,
 		list_add_tail(&sas_info->list, &ioc->sas_device_info_list);
 	}
 	mutex_unlock(&ioc->sas_device_info_mutex);
-
- out:
-	if (buffer)
-		dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
-				  buffer, dma_handle);
+free_dma:
+	dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
+			  buffer, dma_handle);
 }

 /**
@@ -4276,15 +4271,11 @@ mptsas_adding_inactive_raid_components(MPT_ADAPTER *ioc, u8 channel, u8 id)
 	cfg.physAddr = dma_handle;
 	cfg.action = MPI_CONFIG_ACTION_PAGE_READ_CURRENT;

-	if (mpt_config(ioc, &cfg) != 0)
-		goto out;
-
-	if (!(buffer->VolumeStatus.Flags &
-	    MPI_RAIDVOL0_STATUS_FLAG_VOLUME_INACTIVE))
-		goto out;
-
-	if (!buffer->NumPhysDisks)
-		goto out;
+	if (mpt_config(ioc, &cfg) ||
+	    !(buffer->VolumeStatus.Flags &
+	    MPI_RAIDVOL0_STATUS_FLAG_VOLUME_INACTIVE) ||
+	    !buffer->NumPhysDisks)
+		goto free_dma;

 	for (i = 0; i < buffer->NumPhysDisks; i++) {

@@ -4311,11 +4302,9 @@ mptsas_adding_inactive_raid_components(MPT_ADAPTER *ioc, u8 channel, u8 id)
 		    sas_device.sas_address);
 		mptsas_add_end_device(ioc, phy_info);
 	}
-
- out:
-	if (buffer)
-		dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
-				  buffer, dma_handle);
+free_dma:
+	dma_free_coherent(&ioc->pcidev->dev, hdr.PageLength * 4,
+			  buffer, dma_handle);
 }
 /*
  * Work queue thread to handle SAS hotplug events
--
2.40.0


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

* [cocci] [PATCH resent 3/3] scsi: message: fusion: Delete an unnecessary variable initialisation in four functions
  2023-03-25  9:00   ` [cocci] [PATCH resent " Markus Elfring
  2023-03-25  9:03     ` [cocci] [PATCH resent 1/3] scsi: message: fusion: Return directly after input data validation failed in four functions Markus Elfring
  2023-03-25  9:07     ` [cocci] [PATCH resent 2/3] scsi: message: fusion: Delete a redundant pointer check " Markus Elfring
@ 2023-03-25  9:10     ` Markus Elfring
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:10 UTC (permalink / raw)
  To: kernel-janitors, linux-scsi, MPT-FusionLinux.pdl, Sathya Prakash,
	Sreekanth Reddy, Suganath Prabu Subramani
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 16:15:40 +0100

A local variable will eventually be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/message/fusion/mptbase.c | 4 ++--
 drivers/message/fusion/mptsas.c  | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 59420590c5f7..60927c0ee4b8 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -5652,7 +5652,7 @@ mpt_inactive_raid_volumes(MPT_ADAPTER *ioc, u8 channel, u8 id)
 	CONFIGPARMS			cfg;
 	ConfigPageHeader_t		hdr;
 	dma_addr_t			dma_handle;
-	pRaidVolumePage0_t		buffer = NULL;
+	pRaidVolumePage0_t		buffer;
 	int				i;
 	RaidPhysDiskPage0_t 		phys_disk;
 	struct inactive_raid_component_info *component_info;
@@ -6230,7 +6230,7 @@ mpt_get_manufacturing_pg_0(MPT_ADAPTER *ioc)
 	CONFIGPARMS		cfg;
 	ConfigPageHeader_t	hdr;
 	dma_addr_t		buf_dma;
-	ManufacturingPage0_t	*pbuf = NULL;
+	ManufacturingPage0_t	*pbuf;

 	memset(&cfg, 0 , sizeof(CONFIGPARMS));
 	memset(&hdr, 0 , sizeof(ConfigPageHeader_t));
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 5c51339dfb9f..0f8eca1b5ed4 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -682,7 +682,7 @@ mptsas_add_device_component_starget_ir(MPT_ADAPTER *ioc,
 	CONFIGPARMS			cfg;
 	ConfigPageHeader_t		hdr;
 	dma_addr_t			dma_handle;
-	pRaidVolumePage0_t		buffer = NULL;
+	pRaidVolumePage0_t		buffer;
 	int				i;
 	RaidPhysDiskPage0_t 		phys_disk;
 	struct mptsas_device_info	*sas_info, *next;
@@ -4245,7 +4245,7 @@ mptsas_adding_inactive_raid_components(MPT_ADAPTER *ioc, u8 channel, u8 id)
 	CONFIGPARMS			cfg;
 	ConfigPageHeader_t		hdr;
 	dma_addr_t			dma_handle;
-	pRaidVolumePage0_t		buffer = NULL;
+	pRaidVolumePage0_t		buffer;
 	RaidPhysDiskPage0_t 		phys_disk;
 	int				i;
 	struct mptsas_phyinfo	*phy_info;
--
2.40.0


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

* [cocci] [PATCH resent 0/2] md/raid: Adjustments for two function implementations
  2023-03-20 16:47 ` [cocci] [PATCH 0/2] md/raid: Adjustments for two function implementations Markus Elfring
@ 2023-03-25  9:20   ` Markus Elfring
  2023-03-25  9:22     ` [cocci] [PATCH resent 1/2] md/raid1: Fix exception handling in setup_conf() Markus Elfring
                       ` (2 more replies)
  0 siblings, 3 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:20 UTC (permalink / raw)
  To: kernel-janitors, linux-raid, Coly Li, Jens Axboe,
	Kent Overstreet, Maciej Trela, Neil Brown, Shaohua Li, Song Liu
  Cc: cocci, LKML

Date: Mon, 20 Mar 2023 17:28:05 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  raid1: Fix exception handling in setup_conf()
  raid10: Fix exception handling in setup_conf()

 drivers/md/raid1.c  | 75 ++++++++++++++++++++++++++-------------------
 drivers/md/raid10.c | 42 +++++++++++++------------
 2 files changed, 67 insertions(+), 50 deletions(-)

--
2.40.0


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

* [cocci] [PATCH resent 1/2] md/raid1: Fix exception handling in setup_conf()
  2023-03-25  9:20   ` [cocci] [PATCH resent " Markus Elfring
@ 2023-03-25  9:22     ` Markus Elfring
  2023-03-25  9:24     ` [cocci] [PATCH resent 2/2] md/raid10: " Markus Elfring
       [not found]     ` <CAPhsuW7JZDps_fTHyCabjfG4YjzDVEW_41u6d+9mdc2CAJv_Kw@mail.gmail.com>
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:22 UTC (permalink / raw)
  To: kernel-janitors, linux-raid, Coly Li, Jens Axboe,
	Kent Overstreet, Maciej Trela, Neil Brown, Shaohua Li, Song Liu
  Cc: cocci, LKML

Date: Mon, 20 Mar 2023 16:40:11 +0100

The label “abort” was used to jump to another pointer check despite of
the detail in the implementation of the function “setup_conf”
that it was determined already that a corresponding variable contained
still a null pointer because of a failed function call.

1. Thus use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Delete a redundant check.

4. Move three assignments for the variable “err”.


This issue was detected by using the Coccinelle software.

Fixes: 709ae4879ae33628ded276ce7da8cd5acfec476b ("md/raid1: add takeover support for raid5->raid1")
Fixes: fd76863e37fef26fe05547fddfa6e3d05e1682e6 ("RAID1: a new I/O barrier implementation to remove resync window")
Fixes: afeee514ce7f4cab605beedd03be71ebaf0c5fc8 ("md: convert to bioset_init()/mempool_init()")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid1.c | 75 +++++++++++++++++++++++++++-------------------
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 68a9e2d9985b..b614f50505ec 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2978,54 +2978,54 @@ static struct r1conf *setup_conf(struct mddev *mddev)

 	conf = kzalloc(sizeof(struct r1conf), GFP_KERNEL);
 	if (!conf)
-		goto abort;
+		goto exit;

 	conf->nr_pending = kcalloc(BARRIER_BUCKETS_NR,
 				   sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->nr_pending)
-		goto abort;
+		goto free_conf;

 	conf->nr_waiting = kcalloc(BARRIER_BUCKETS_NR,
 				   sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->nr_waiting)
-		goto abort;
+		goto free_pending;

 	conf->nr_queued = kcalloc(BARRIER_BUCKETS_NR,
 				  sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->nr_queued)
-		goto abort;
+		goto free_waiting;

 	conf->barrier = kcalloc(BARRIER_BUCKETS_NR,
 				sizeof(atomic_t), GFP_KERNEL);
 	if (!conf->barrier)
-		goto abort;
+		goto free_queued;

 	conf->mirrors = kzalloc(array3_size(sizeof(struct raid1_info),
 					    mddev->raid_disks, 2),
 				GFP_KERNEL);
 	if (!conf->mirrors)
-		goto abort;
+		goto free_barrier;

 	conf->tmppage = alloc_page(GFP_KERNEL);
 	if (!conf->tmppage)
-		goto abort;
+		goto free_mirrors;

 	conf->poolinfo = kzalloc(sizeof(*conf->poolinfo), GFP_KERNEL);
 	if (!conf->poolinfo)
-		goto abort;
+		goto put_page;
+
 	conf->poolinfo->raid_disks = mddev->raid_disks * 2;
 	err = mempool_init(&conf->r1bio_pool, NR_RAID_BIOS, r1bio_pool_alloc,
 			   rbio_pool_free, conf->poolinfo);
 	if (err)
-		goto abort;
+		goto free_poolinfo;

 	err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
 	if (err)
-		goto abort;
+		goto mempool_exit;

 	conf->poolinfo->mddev = mddev;

-	err = -EINVAL;
 	spin_lock_init(&conf->device_lock);
 	rdev_for_each(rdev, mddev) {
 		int disk_idx = rdev->raid_disk;
@@ -3037,8 +3037,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 		else
 			disk = conf->mirrors + disk_idx;

-		if (disk->rdev)
-			goto abort;
+		if (disk->rdev) {
+			err = -EINVAL;
+			goto bioset_exit;
+		}
+
 		disk->rdev = rdev;
 		disk->head_position = 0;
 		disk->seq_start = MaxSector;
@@ -3054,7 +3057,6 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 	bio_list_init(&conf->pending_bio_list);
 	conf->recovery_disabled = mddev->recovery_disabled - 1;

-	err = -EIO;
 	for (i = 0; i < conf->raid_disks * 2; i++) {

 		disk = conf->mirrors + i;
@@ -3069,9 +3071,11 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 				disk->rdev =
 					disk[conf->raid_disks].rdev;
 				disk[conf->raid_disks].rdev = NULL;
-			} else if (!test_bit(In_sync, &disk->rdev->flags))
+			} else if (!test_bit(In_sync, &disk->rdev->flags)) {
 				/* Original is not in_sync - bad */
-				goto abort;
+				err = -EIO;
+				goto bioset_exit;
+			}
 		}

 		if (!disk->rdev ||
@@ -3083,26 +3087,35 @@ static struct r1conf *setup_conf(struct mddev *mddev)
 		}
 	}

-	err = -ENOMEM;
 	conf->thread = md_register_thread(raid1d, mddev, "raid1");
 	if (!conf->thread)
-		goto abort;
+		goto e_nomem;

 	return conf;

- abort:
-	if (conf) {
-		mempool_exit(&conf->r1bio_pool);
-		kfree(conf->mirrors);
-		safe_put_page(conf->tmppage);
-		kfree(conf->poolinfo);
-		kfree(conf->nr_pending);
-		kfree(conf->nr_waiting);
-		kfree(conf->nr_queued);
-		kfree(conf->barrier);
-		bioset_exit(&conf->bio_split);
-		kfree(conf);
-	}
+e_nomem:
+	err = -ENOMEM;
+bioset_exit:
+	bioset_exit(&conf->bio_split);
+mempool_exit:
+	mempool_exit(&conf->r1bio_pool);
+free_poolinfo:
+	kfree(conf->poolinfo);
+put_page:
+	safe_put_page(conf->tmppage);
+free_mirrors:
+	kfree(conf->mirrors);
+free_barrier:
+	kfree(conf->barrier);
+free_queued:
+	kfree(conf->nr_queued);
+free_waiting:
+	kfree(conf->nr_waiting);
+free_pending:
+	kfree(conf->nr_pending);
+free_conf:
+	kfree(conf);
+exit:
 	return ERR_PTR(err);
 }

--
2.40.0


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

* [cocci] [PATCH resent 2/2] md/raid10: Fix exception handling in setup_conf()
  2023-03-25  9:20   ` [cocci] [PATCH resent " Markus Elfring
  2023-03-25  9:22     ` [cocci] [PATCH resent 1/2] md/raid1: Fix exception handling in setup_conf() Markus Elfring
@ 2023-03-25  9:24     ` Markus Elfring
       [not found]     ` <CAPhsuW7JZDps_fTHyCabjfG4YjzDVEW_41u6d+9mdc2CAJv_Kw@mail.gmail.com>
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:24 UTC (permalink / raw)
  To: kernel-janitors, linux-raid, Coly Li, Jens Axboe,
	Kent Overstreet, Maciej Trela, Neil Brown, Shaohua Li, Song Liu
  Cc: cocci, LKML

Date: Mon, 20 Mar 2023 17:15:07 +0100

The label “out” was used to jump to another pointer check despite of
the detail in the implementation of the function “setup_conf”
that it was determined already that a corresponding variable contained
still a null pointer because of a failed function call.

1. Thus use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Delete a redundant check.

4. Move an assignment for the variable “err”.

5. Omit an extra initialisation (for the variable “conf”)
   which became unnecessary with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: dab8b29248b3f14f456651a2a6ee9b8fd16d1b3c ("md: Add support for Raid0->Raid10 takeover")
Fixes: afeee514ce7f4cab605beedd03be71ebaf0c5fc8 ("md: convert to bioset_init()/mempool_init()")
Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/raid10.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 6c66357f92f5..12ce3e69e6f7 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4006,7 +4006,7 @@ static int setup_geo(struct geom *geo, struct mddev *mddev, enum geo_type new)

 static struct r10conf *setup_conf(struct mddev *mddev)
 {
-	struct r10conf *conf = NULL;
+	struct r10conf *conf;
 	int err = -EINVAL;
 	struct geom geo;
 	int copies;
@@ -4016,41 +4016,41 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	if (copies == -2) {
 		pr_warn("md/raid10:%s: chunk size must be at least PAGE_SIZE(%ld) and be a power of 2.\n",
 			mdname(mddev), PAGE_SIZE);
-		goto out;
+		goto exit;
 	}

 	if (copies < 2 || copies > mddev->raid_disks) {
 		pr_warn("md/raid10:%s: unsupported raid10 layout: 0x%8x\n",
 			mdname(mddev), mddev->new_layout);
-		goto out;
+		goto exit;
 	}

 	err = -ENOMEM;
 	conf = kzalloc(sizeof(struct r10conf), GFP_KERNEL);
 	if (!conf)
-		goto out;
+		goto exit;

 	/* FIXME calc properly */
 	conf->mirrors = kcalloc(mddev->raid_disks + max(0, -mddev->delta_disks),
 				sizeof(struct raid10_info),
 				GFP_KERNEL);
 	if (!conf->mirrors)
-		goto out;
+		goto free_conf;

 	conf->tmppage = alloc_page(GFP_KERNEL);
 	if (!conf->tmppage)
-		goto out;
+		goto free_mirrors;

 	conf->geo = geo;
 	conf->copies = copies;
 	err = mempool_init(&conf->r10bio_pool, NR_RAID_BIOS, r10bio_pool_alloc,
 			   rbio_pool_free, conf);
 	if (err)
-		goto out;
+		goto put_page;

 	err = bioset_init(&conf->bio_split, BIO_POOL_SIZE, 0, 0);
 	if (err)
-		goto out;
+		goto mempool_exit;

 	calc_sectors(conf, mddev->dev_sectors);
 	if (mddev->reshape_position == MaxSector) {
@@ -4059,7 +4059,7 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	} else {
 		if (setup_geo(&conf->prev, mddev, geo_old) != conf->copies) {
 			err = -EINVAL;
-			goto out;
+			goto bioset_exit;
 		}
 		conf->reshape_progress = mddev->reshape_position;
 		if (conf->prev.far_offset)
@@ -4077,22 +4077,26 @@ static struct r10conf *setup_conf(struct mddev *mddev)
 	init_waitqueue_head(&conf->wait_barrier);
 	atomic_set(&conf->nr_pending, 0);

-	err = -ENOMEM;
 	conf->thread = md_register_thread(raid10d, mddev, "raid10");
 	if (!conf->thread)
-		goto out;
+		goto e_nomem;

 	conf->mddev = mddev;
 	return conf;

- out:
-	if (conf) {
-		mempool_exit(&conf->r10bio_pool);
-		kfree(conf->mirrors);
-		safe_put_page(conf->tmppage);
-		bioset_exit(&conf->bio_split);
-		kfree(conf);
-	}
+e_nomem:
+	err = -ENOMEM;
+bioset_exit:
+	bioset_exit(&conf->bio_split);
+mempool_exit:
+	mempool_exit(&conf->r10bio_pool);
+put_page:
+	safe_put_page(conf->tmppage);
+free_mirrors:
+	kfree(conf->mirrors);
+free_conf:
+	kfree(conf);
+exit:
 	return ERR_PTR(err);
 }

--
2.40.0


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

* [cocci] [PATCH resent] bcache: Fix exception handling in mca_alloc()
       [not found] ` <e33f264a-7ee9-4ebc-d58e-bbb7fd567198@web.de>
@ 2023-03-25  9:31   ` Markus Elfring
       [not found]     ` <157b8db9-82f7-85e7-3bbd-7ef3a1797892@suse.de>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:31 UTC (permalink / raw)
  To: kernel-janitors, linux-bcache, Coly Li, Kent Overstreet; +Cc: cocci, LKML

Date: Mon, 20 Mar 2023 13:13:37 +0100

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “mca_alloc”
that it was determined already that a corresponding variable contained
a null pointer because of a failed function call “mca_bucket_alloc”.

* Thus use a more appropriate label instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: cafe563591446cf80bfbc2fe3bc72a2e36cf1060 ("bcache: A block layer cache")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/md/bcache/btree.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 147c493a989a..166afd7ec499 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -921,18 +921,18 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
 		if (!mca_reap(b, 0, false)) {
 			mca_data_alloc(b, k, __GFP_NOWARN|GFP_NOIO);
 			if (!b->keys.set[0].data)
-				goto err;
+				goto unlock;
 			else
 				goto out;
 		}

 	b = mca_bucket_alloc(c, k, __GFP_NOWARN|GFP_NOIO);
 	if (!b)
-		goto err;
+		goto unlock;

 	BUG_ON(!down_write_trylock(&b->lock));
 	if (!b->keys.set->data)
-		goto err;
+		goto unlock;
 out:
 	BUG_ON(b->io_mutex.count != 1);

@@ -955,9 +955,8 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
 				    &b->c->expensive_debug_checks);

 	return b;
-err:
-	if (b)
-		rw_unlock(true, b);
+unlock:
+	rw_unlock(true, b);

 	b = mca_cannibalize(c, op, k);
 	if (!IS_ERR(b))
--
2.40.0


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

* [cocci] [PATCH resent] mei: Fix exception handling in mei_cl_irq_read_msg()
       [not found] ` <00589154-00ac-4ed5-2a37-60b3c6f6c523@web.de>
@ 2023-03-25  9:40   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:40 UTC (permalink / raw)
  To: kernel-janitors, Alexander Usyskin, Arnd Bergmann,
	Greg Kroah-Hartman, Tomas Winkler
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 18:11:13 +0100

The label “discard” was used to jump to another pointer check despite of
the detail in the implementation of the function “mei_cl_irq_read_msg”
that it was determined already that a corresponding variable contained
a null pointer.

* Thus use an additional label instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: a808c80cdaa83939b220176fcdffca8385d88ba6 ("mei: add read callback on demand for fixed_address clients")
Fixes: 17ba8a08b58a01bbac35790ffca4388ca92b7790 ("mei: consolidate repeating code in mei_cl_irq_read_msg")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/misc/mei/interrupt.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/mei/interrupt.c b/drivers/misc/mei/interrupt.c
index 0a0e984e5673..9800d30b7693 100644
--- a/drivers/misc/mei/interrupt.c
+++ b/drivers/misc/mei/interrupt.c
@@ -136,7 +136,7 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 				cb->ext_hdr = kzalloc(sizeof(*gsc_f2h), GFP_KERNEL);
 				if (!cb->ext_hdr) {
 					cb->status = -ENOMEM;
-					goto discard;
+					goto move_tail;
 				}
 				break;
 			case MEI_EXT_HDR_NONE:
@@ -153,7 +153,7 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 		if (!vtag_hdr && !gsc_f2h) {
 			cl_dbg(dev, cl, "no vtag or gsc found in extended header.\n");
 			cb->status = -EPROTO;
-			goto discard;
+			goto move_tail;
 		}
 	}

@@ -163,7 +163,7 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 			cl_err(dev, cl, "mismatched tag: %d != %d\n",
 			       cb->vtag, vtag_hdr->vtag);
 			cb->status = -EPROTO;
-			goto discard;
+			goto move_tail;
 		}
 		cb->vtag = vtag_hdr->vtag;
 	}
@@ -174,18 +174,18 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 		if (!dev->hbm_f_gsc_supported) {
 			cl_err(dev, cl, "gsc extended header is not supported\n");
 			cb->status = -EPROTO;
-			goto discard;
+			goto move_tail;
 		}

 		if (length) {
 			cl_err(dev, cl, "no data allowed in cb with gsc\n");
 			cb->status = -EPROTO;
-			goto discard;
+			goto move_tail;
 		}
 		if (ext_hdr_len > sizeof(*gsc_f2h)) {
 			cl_err(dev, cl, "gsc extended header is too big %u\n", ext_hdr_len);
 			cb->status = -EPROTO;
-			goto discard;
+			goto move_tail;
 		}
 		memcpy(cb->ext_hdr, gsc_f2h, ext_hdr_len);
 	}
@@ -193,7 +193,7 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 	if (!mei_cl_is_connected(cl)) {
 		cl_dbg(dev, cl, "not connected\n");
 		cb->status = -ENODEV;
-		goto discard;
+		goto move_tail;
 	}

 	if (mei_hdr->dma_ring)
@@ -205,14 +205,14 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,
 		cl_err(dev, cl, "message is too big len %d idx %zu\n",
 		       length, cb->buf_idx);
 		cb->status = -EMSGSIZE;
-		goto discard;
+		goto move_tail;
 	}

 	if (cb->buf.size < buf_sz) {
 		cl_dbg(dev, cl, "message overflow. size %zu len %d idx %zu\n",
 			cb->buf.size, length, cb->buf_idx);
 		cb->status = -EMSGSIZE;
-		goto discard;
+		goto move_tail;
 	}

 	if (mei_hdr->dma_ring) {
@@ -235,9 +235,9 @@ static int mei_cl_irq_read_msg(struct mei_cl *cl,

 	return 0;

+move_tail:
+	list_move_tail(&cb->list, cmpl_list);
 discard:
-	if (cb)
-		list_move_tail(&cb->list, cmpl_list);
 	mei_irq_discard_msg(dev, mei_hdr, length);
 	return 0;
 }
--
2.40.0


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

* [cocci] [PATCH resent] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup()
       [not found] ` <3675f707-bff0-3caf-29a2-b99e5b9c6554@web.de>
@ 2023-03-25  9:43   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:43 UTC (permalink / raw)
  To: kernel-janitors, linux-mtd, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 20:13:51 +0100

The label “setup_err” was used to jump to another pointer check despite of
the detail in the implementation of the function “cfi_intelext_setup”
that it was determined already that a corresponding variable contained
a null pointer because of a failed memory allocation.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mtd/chips/cfi_cmdset_0001.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
index 54f92d09d9cf..a06318cd5ea4 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -614,7 +614,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 				    sizeof(struct mtd_erase_region_info),
 				    GFP_KERNEL);
 	if (!mtd->eraseregions)
-		goto setup_err;
+		goto free_mtd;

 	for (i=0; i<cfi->cfiq->NumEraseRegions; i++) {
 		unsigned long ernum, ersize;
@@ -630,7 +630,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].numblocks = ernum;
 			mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap = kmalloc(ernum / 8 + 1, GFP_KERNEL);
 			if (!mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap)
-				goto setup_err;
+				goto release_loop;
 		}
 		offset += (ersize * ernum);
 	}
@@ -638,7 +638,7 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	if (offset != devsize) {
 		/* Argh */
 		printk(KERN_WARNING "Sum of regions (%lx) != total size of set of interleaved chips (%lx)\n", offset, devsize);
-		goto setup_err;
+		goto release_loop;
 	}

 	for (i=0; i<mtd->numeraseregions;i++){
@@ -660,18 +660,18 @@ static struct mtd_info *cfi_intelext_setup(struct mtd_info *mtd)
 	/* This function has the potential to distort the reality
 	   a bit and therefore should be called last. */
 	if (cfi_intelext_partition_fixup(mtd, &cfi) != 0)
-		goto setup_err;
+		goto release_loop;

 	__module_get(THIS_MODULE);
 	register_reboot_notifier(&mtd->reboot_notifier);
 	return mtd;

- setup_err:
-	if (mtd->eraseregions)
-		for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
-			for (j=0; j<cfi->numchips; j++)
-				kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
+release_loop:
+	for (i=0; i<cfi->cfiq->NumEraseRegions; i++)
+		for (j=0; j<cfi->numchips; j++)
+			kfree(mtd->eraseregions[(j*cfi->cfiq->NumEraseRegions)+i].lockmap);
 	kfree(mtd->eraseregions);
+free_mtd:
 	kfree(mtd);
 	kfree(cfi->cmdset_priv);
 	return NULL;
--
2.40.0


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

* [cocci] [PATCH resent] bbc_i2c: Fix exception handling in attach_one_i2c()
       [not found] ` <21e58abb-f215-b9b7-ffe8-236dd40c6bd2@web.de>
@ 2023-03-25  9:50   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25  9:50 UTC (permalink / raw)
  To: kernel-janitors, sparclinux,
	Christopher Alexander Tobias Schulze, David S. Miller
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 21:12:29 +0100

The label “fail” was used to jump to another pointer check despite of
the detail in the implementation of the function “attach_one_i2c”
that it was determined already that a corresponding variable contained
a null pointer because of a failed call of the function “of_ioremap”.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 5cdceab3d5e02eb69ea0f5d8fa9181800baf6f77 ("bbc-i2c: Fix BBC I2C envctrl on SunBlade 2000")
Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/sbus/char/bbc_i2c.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/sbus/char/bbc_i2c.c b/drivers/sbus/char/bbc_i2c.c
index 537e55cd038d..03e29f2760b2 100644
--- a/drivers/sbus/char/bbc_i2c.c
+++ b/drivers/sbus/char/bbc_i2c.c
@@ -306,19 +306,19 @@ static struct bbc_i2c_bus * attach_one_i2c(struct platform_device *op, int index

 	bp->i2c_control_regs = of_ioremap(&op->resource[0], 0, 0x2, "bbc_i2c_regs");
 	if (!bp->i2c_control_regs)
-		goto fail;
+		goto free_bus;

 	if (op->num_resources == 2) {
 		bp->i2c_bussel_reg = of_ioremap(&op->resource[1], 0, 0x1, "bbc_i2c_bussel");
 		if (!bp->i2c_bussel_reg)
-			goto fail;
+			goto unmap_io_control_regs;
 	}

 	bp->waiting = 0;
 	init_waitqueue_head(&bp->wq);
 	if (request_irq(op->archdata.irqs[0], bbc_i2c_interrupt,
 			IRQF_SHARED, "bbc_i2c", bp))
-		goto fail;
+		goto recheck_bussel_reg;

 	bp->index = index;
 	bp->op = op;
@@ -348,11 +348,12 @@ static struct bbc_i2c_bus * attach_one_i2c(struct platform_device *op, int index

 	return bp;

-fail:
+recheck_bussel_reg:
 	if (bp->i2c_bussel_reg)
 		of_iounmap(&op->resource[1], bp->i2c_bussel_reg, 1);
-	if (bp->i2c_control_regs)
-		of_iounmap(&op->resource[0], bp->i2c_control_regs, 2);
+unmap_io_control_regs:
+	of_iounmap(&op->resource[0], bp->i2c_control_regs, 2);
+free_bus:
 	kfree(bp);
 	return NULL;
 }
--
2.40.0


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

* [cocci] [PATCH resent] Input: iforce - Fix exception handling in iforce_usb_probe()
       [not found]       ` <ZBf3oSgJP8bLmhG0@google.com>
@ 2023-03-25 10:12         ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 10:12 UTC (permalink / raw)
  To: kernel-janitors, linux-input, Dmitry Torokhov, Hillf Danton,
	Tetsuo Handa
  Cc: cocci, LKML

Date: Sun, 19 Mar 2023 18:50:51 +0100

The label “fail” was used to jump to another pointer check despite of
the detail in the implementation of the function “iforce_usb_probe”
that it was determined already that a corresponding variable contained
still a null pointer.

1. Use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 81fd43132684605b21600fa5e27f23034e18dfd3 ("Input: iforce - move transport data into transport modules")
Fixes: ^1da177e4c3f41524e886b7f1b8a0c1fc7321cac ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/input/joystick/iforce/iforce-usb.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/input/joystick/iforce/iforce-usb.c b/drivers/input/joystick/iforce/iforce-usb.c
index cba92bd590a8..f4d70831adf1 100644
--- a/drivers/input/joystick/iforce/iforce-usb.c
+++ b/drivers/input/joystick/iforce/iforce-usb.c
@@ -210,15 +210,15 @@ static int iforce_usb_probe(struct usb_interface *intf,

 	iforce_usb = kzalloc(sizeof(*iforce_usb), GFP_KERNEL);
 	if (!iforce_usb)
-		goto fail;
+		goto exit;

 	iforce_usb->irq = usb_alloc_urb(0, GFP_KERNEL);
 	if (!iforce_usb->irq)
-		goto fail;
+		goto free_usb;

 	iforce_usb->out = usb_alloc_urb(0, GFP_KERNEL);
 	if (!iforce_usb->out)
-		goto fail;
+		goto free_urb_irq;

 	iforce_usb->iforce.xport_ops = &iforce_usb_xport_ops;

@@ -237,18 +237,18 @@ static int iforce_usb_probe(struct usb_interface *intf,

 	err = iforce_init_device(&intf->dev, BUS_USB, &iforce_usb->iforce);
 	if (err)
-		goto fail;
+		goto free_urb_out;

 	usb_set_intfdata(intf, iforce_usb);
 	return 0;

-fail:
-	if (iforce_usb) {
-		usb_free_urb(iforce_usb->irq);
-		usb_free_urb(iforce_usb->out);
-		kfree(iforce_usb);
-	}
-
+free_urb_out:
+	usb_free_urb(iforce_usb->out);
+free_urb_irq:
+	usb_free_urb(iforce_usb->irq);
+free_usb:
+	kfree(iforce_usb);
+exit:
 	return err;
 }

--
2.40.0


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

* [cocci] [PATCH resent] irqchip/partitions: Fix exception handling in partition_create_desc()
       [not found] ` <15fa53e5-916f-dac8-87fb-9cb81021418c@web.de>
@ 2023-03-25 10:20   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 10:20 UTC (permalink / raw)
  To: kernel-janitors, Marc Zyngier, Thomas Gleixner; +Cc: cocci, LKML

Date: Sun, 19 Mar 2023 21:38:47 +0100

The label “out” was used to jump to another pointer check despite of
the detail in the implementation of the function “partition_create_desc”
that it was determined already that a corresponding variable contained
still a null pointer.

* Thus use more appropriate labels instead.

* Delete a redundant check.


This issue was detected by using the Coccinelle software.

Fixes: 9e2c986cb460bf97154f18e85aa833739a1e8dc7 ("irqchip: Add per-cpu interrupt partitioning library")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/irqchip/irq-partition-percpu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-partition-percpu.c b/drivers/irqchip/irq-partition-percpu.c
index 8e76d2913e6b..80e1c73af87e 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -212,21 +212,21 @@ struct partition_desc *partition_create_desc(struct fwnode_handle *fwnode,

 	d = irq_domain_create_linear(fwnode, nr_parts, &desc->ops, desc);
 	if (!d)
-		goto out;
+		goto free_desc;
 	desc->domain = d;

 	desc->bitmap = bitmap_zalloc(nr_parts, GFP_KERNEL);
 	if (WARN_ON(!desc->bitmap))
-		goto out;
+		goto remove_domain;

 	desc->chained_desc = irq_to_desc(chained_irq);
 	desc->nr_parts = nr_parts;
 	desc->parts = parts;

 	return desc;
-out:
-	if (d)
-		irq_domain_remove(d);
+remove_domain:
+	irq_domain_remove(d);
+free_desc:
 	kfree(desc);

 	return NULL;
--
2.40.0


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

* [cocci] [PATCH resent 0/2] irqchip/gic-v4: Adjustments for two function implementations
  2023-03-19 20:00 ` [cocci] [PATCH 0/2] irqchip/gic-v4: Adjustments for two function implementations Markus Elfring
@ 2023-03-25 10:25   ` Markus Elfring
  2023-03-25 10:27     ` [cocci] [PATCH resent 1/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_irqs() Markus Elfring
  2023-03-25 10:30     ` [cocci] [PATCH resent 2/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_sgis() Markus Elfring
  0 siblings, 2 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 10:25 UTC (permalink / raw)
  To: kernel-janitors, Marc Zyngier, Thomas Gleixner, Zenghui Yu; +Cc: cocci, LKML

Date: Sun, 19 Mar 2023 20:54:05 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Fix exception handling in its_alloc_vcpu_irqs()
  Fix exception handling in its_alloc_vcpu_sgis()

 drivers/irqchip/irq-gic-v4.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

--
2.40.0


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

* [cocci] [PATCH resent 1/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_irqs()
  2023-03-25 10:25   ` [cocci] [PATCH resent " Markus Elfring
@ 2023-03-25 10:27     ` Markus Elfring
  2023-03-25 10:30     ` [cocci] [PATCH resent 2/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_sgis() Markus Elfring
  1 sibling, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 10:27 UTC (permalink / raw)
  To: kernel-janitors, Marc Zyngier, Thomas Gleixner, Zenghui Yu; +Cc: cocci, LKML

Date: Sun, 19 Mar 2023 20:05:21 +0100

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “its_alloc_vcpu_irqs”
that it was determined already that a corresponding variable contained
still a null pointer.

Use more appropriate labels instead.

This issue was detected by using the Coccinelle software.

Fixes: 7de5c0af9c7c717f9052e6d75b24f90050e6a56e ("irqchip/gic-v4: Add per-VM VPE domain creation")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/irqchip/irq-gic-v4.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 94d56a03b175..d98d58298e9e 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -161,13 +161,13 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
 	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv4-vpe",
 						      task_pid_nr(current));
 	if (!vm->fwnode)
-		goto err;
+		goto recheck_domain;

 	vm->domain = irq_domain_create_hierarchy(gic_domain, 0, vm->nr_vpes,
 						 vm->fwnode, vpe_domain_ops,
 						 vm);
 	if (!vm->domain)
-		goto err;
+		goto free_fwnode;

 	for (i = 0; i < vm->nr_vpes; i++) {
 		vm->vpes[i]->its_vm = vm;
@@ -177,22 +177,25 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
 	vpe_base_irq = irq_domain_alloc_irqs(vm->domain, vm->nr_vpes,
 					     NUMA_NO_NODE, vm);
 	if (vpe_base_irq <= 0)
-		goto err;
+		goto remove_domain;

 	for (i = 0; i < vm->nr_vpes; i++) {
 		int ret;
 		vm->vpes[i]->irq = vpe_base_irq + i;
 		ret = its_alloc_vcpu_sgis(vm->vpes[i], i);
 		if (ret)
-			goto err;
+			goto remove_domain;
 	}

 	return 0;

-err:
+recheck_domain:
 	if (vm->domain)
+remove_domain:
 		irq_domain_remove(vm->domain);
+
 	if (vm->fwnode)
+free_fwnode:
 		irq_domain_free_fwnode(vm->fwnode);

 	return -ENOMEM;
--
2.40.0


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

* [cocci] [PATCH resent 2/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_sgis()
  2023-03-25 10:25   ` [cocci] [PATCH resent " Markus Elfring
  2023-03-25 10:27     ` [cocci] [PATCH resent 1/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_irqs() Markus Elfring
@ 2023-03-25 10:30     ` Markus Elfring
  1 sibling, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 10:30 UTC (permalink / raw)
  To: kernel-janitors, Marc Zyngier, Thomas Gleixner, Zenghui Yu; +Cc: cocci, LKML

Date: Sun, 19 Mar 2023 20:28:52 +0100

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “its_alloc_vcpu_sgis”
that it was determined already that a corresponding variable contained
still a null pointer.

1. Thus return directly after a call of the function “kasprintf” failed.

2. Use more appropriate labels instead.

3. Delete two questionable checks.


This issue was detected by using the Coccinelle software.

Fixes: 6d31b6ff985dbd144b2c4d519cf573b8f81865d9 ("irqchip/gic-v4.1: Add VSGI allocation/teardown")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/irqchip/irq-gic-v4.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index d98d58298e9e..f68560bfd094 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -125,11 +125,11 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)

 	name = kasprintf(GFP_KERNEL, "GICv4-sgi-%d", task_pid_nr(current));
 	if (!name)
-		goto err;
+		return -ENOMEM;

 	vpe->fwnode = irq_domain_alloc_named_id_fwnode(name, idx);
 	if (!vpe->fwnode)
-		goto err;
+		goto free_name;

 	kfree(name);
 	name = NULL;
@@ -137,19 +137,19 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
 	vpe->sgi_domain = irq_domain_create_linear(vpe->fwnode, 16,
 						   sgi_domain_ops, vpe);
 	if (!vpe->sgi_domain)
-		goto err;
+		goto free_fwnode;

 	sgi_base = irq_domain_alloc_irqs(vpe->sgi_domain, 16, NUMA_NO_NODE, vpe);
 	if (sgi_base <= 0)
-		goto err;
+		goto remove_domain;

 	return 0;

-err:
-	if (vpe->sgi_domain)
-		irq_domain_remove(vpe->sgi_domain);
-	if (vpe->fwnode)
-		irq_domain_free_fwnode(vpe->fwnode);
+remove_domain:
+	irq_domain_remove(vpe->sgi_domain);
+free_fwnode:
+	irq_domain_free_fwnode(vpe->fwnode);
+free_name:
 	kfree(name);
 	return -ENOMEM;
 }
--
2.40.0


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

* [cocci] [PATCH v2] bcache: Fix exception handling in mca_alloc()
       [not found]     ` <157b8db9-82f7-85e7-3bbd-7ef3a1797892@suse.de>
@ 2023-03-25 12:21       ` Markus Elfring
       [not found]         ` <BE6CEE57-E9AF-4F17-B281-1E00C5DC2A9C@suse.de>
  2023-03-25 12:50       ` [cocci] [PATCH resent] " Markus Elfring
  1 sibling, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 12:21 UTC (permalink / raw)
  To: kernel-janitors, linux-bcache, Coly Li, Kent Overstreet; +Cc: cocci, LKML

Date: Sat, 25 Mar 2023 13:08:01 +0100

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “mca_alloc”
that it was determined already that a corresponding variable contained
a null pointer because of a failed function call “mca_bucket_alloc”.

1. Thus use more appropriate labels instead.

2. Delete a repeated check (for the variable “b”)
   which became unnecessary with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: cafe563591446cf80bfbc2fe3bc72a2e36cf1060 ("bcache: A block layer cache")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
V2:
Use another label.

 drivers/md/bcache/btree.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c
index 147c493a989a..c6a20595302f 100644
--- a/drivers/md/bcache/btree.c
+++ b/drivers/md/bcache/btree.c
@@ -921,18 +921,18 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
 		if (!mca_reap(b, 0, false)) {
 			mca_data_alloc(b, k, __GFP_NOWARN|GFP_NOIO);
 			if (!b->keys.set[0].data)
-				goto err;
+				goto unlock;
 			else
 				goto out;
 		}

 	b = mca_bucket_alloc(c, k, __GFP_NOWARN|GFP_NOIO);
 	if (!b)
-		goto err;
+		goto cannibalize_mca;

 	BUG_ON(!down_write_trylock(&b->lock));
 	if (!b->keys.set->data)
-		goto err;
+		goto unlock;
 out:
 	BUG_ON(b->io_mutex.count != 1);

@@ -955,10 +955,9 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
 				    &b->c->expensive_debug_checks);

 	return b;
-err:
-	if (b)
-		rw_unlock(true, b);
-
+unlock:
+	rw_unlock(true, b);
+cannibalize_mca:
 	b = mca_cannibalize(c, op, k);
 	if (!IS_ERR(b))
 		goto out;
--
2.40.0


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

* Re: [cocci] [PATCH resent] bcache: Fix exception handling in mca_alloc()
       [not found]     ` <157b8db9-82f7-85e7-3bbd-7ef3a1797892@suse.de>
  2023-03-25 12:21       ` [cocci] [PATCH v2] " Markus Elfring
@ 2023-03-25 12:50       ` Markus Elfring
  1 sibling, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 12:50 UTC (permalink / raw)
  To: Coly Li, linux-bcache, kernel-janitors; +Cc: cocci, LKML, Kent Overstreet

>> The label “err” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “mca_alloc”
>> that it was determined already that a corresponding variable contained
>> a null pointer because of a failed function call “mca_bucket_alloc”.
>
> Hmm, I don't get the exact point from the above long sentence.

Would you find another description variant helpful for the understanding
of an applied source code search pattern?

Reconsidering repeated pointer checks with SmPL
https://lore.kernel.org/cocci/f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00017.html


>> * Thus use a more appropriate label instead.
>
> So far I am not convinced the modified label is more appropriate.

Can the usage of specific goto targets become more appealing?


>> * Delete a redundant check.
>
> Where is the location of the redundant check?

I propose to avoid the repetition of a pointer check at this place:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/md/bcache/btree.c?h=v6.3-rc3#n959


>> @@ -955,9 +955,8 @@ static struct btree *mca_alloc(struct cache_set *c, struct btree_op *op,
>>                       &b->c->expensive_debug_checks);
>>
>>       return b;
>> -err:
>> -    if (b)
>> -        rw_unlock(true, b);
>> +unlock:
>> +    rw_unlock(true, b);
>
> If b is NULL, is it a bit fishing to send the NULL pointer into rw_unlock() ?

Thanks for your reminder.

I accordingly sent the next patch version a moment ago.
https://lore.kernel.org/cocci/13b4a57a-5911-16db-2b6e-588e5137c3aa@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00068.html

Regards,
Markus

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

* [cocci] [PATCH resent v2 0/2] powerpc/pseries: Fixes for exception handling in pSeries_reconfig_add_node()
  2023-03-21 10:30               ` [cocci] [PATCH v2 0/2] powerpc/pseries: Fixes for " Markus Elfring
@ 2023-03-25 13:40                 ` Markus Elfring
  2023-03-25 13:42                   ` [cocci] [PATCH resent v2 1/2] powerpc/pseries: Do not pass an error pointer to of_node_put() " Markus Elfring
                                     ` (2 more replies)
  0 siblings, 3 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 13:40 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Christophe Leroy,
	Michael Ellerman, Nathan Lynch, Nicholas Piggin, Paul Moore
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 11:26:32 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Do not pass an error pointer to of_node_put()
  Fix exception handling

 arch/powerpc/platforms/pseries/reconfig.c | 26 ++++++++++++-----------
 1 file changed, 14 insertions(+), 12 deletions(-)

--
2.40.0


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

* [cocci] [PATCH resent v2 1/2] powerpc/pseries: Do not pass an error pointer to of_node_put() in pSeries_reconfig_add_node()
  2023-03-25 13:40                 ` [cocci] [PATCH resent " Markus Elfring
@ 2023-03-25 13:42                   ` Markus Elfring
  2023-03-25 13:44                   ` [cocci] [PATCH resent v2 2/2] powerpc/pseries: Fix exception handling " Markus Elfring
  2024-01-05 17:19                   ` [cocci] [PATCH resent v2 0/2] powerpc/pseries: Fixes for " Markus Elfring
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 13:42 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Christophe Leroy,
	Michael Ellerman, Nathan Lynch, Nicholas Piggin, Paul Moore
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 10:30:23 +0100

It can be determined in the implementation of the function
“pSeries_reconfig_add_node” that an error code would occasionally
be provided by a call of a function like pseries_of_derive_parent().
This error indication was passed to an of_node_put() call according to
an attempt for exception handling so far.

Thus fix the risk for undesirable software behaviour by using
an additional label for this error case.

Link: https://lists.ozlabs.org/pipermail/linuxppc-dev/2023-March/256025.html
Link: https://lore.kernel.org/lkml/87pm9377qt.fsf@linux.ibm.com/
Reported-by: Nathan Lynch <nathanl@linux.ibm.com>
Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
V2:
This update step was added according to another change request.

 arch/powerpc/platforms/pseries/reconfig.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 599bd2c78514..44f8ebc2ec0d 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -40,7 +40,7 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
 	np->parent = pseries_of_derive_parent(path);
 	if (IS_ERR(np->parent)) {
 		err = PTR_ERR(np->parent);
-		goto out_err;
+		goto free_name;
 	}

 	err = of_attach_node(np);
@@ -56,6 +56,7 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
 out_err:
 	if (np) {
 		of_node_put(np->parent);
+free_name:
 		kfree(np->full_name);
 		kfree(np);
 	}
--
2.40.0


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

* [cocci] [PATCH resent v2 2/2] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node()
  2023-03-25 13:40                 ` [cocci] [PATCH resent " Markus Elfring
  2023-03-25 13:42                   ` [cocci] [PATCH resent v2 1/2] powerpc/pseries: Do not pass an error pointer to of_node_put() " Markus Elfring
@ 2023-03-25 13:44                   ` Markus Elfring
  2024-01-05 17:19                   ` [cocci] [PATCH resent v2 0/2] powerpc/pseries: Fixes for " Markus Elfring
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 13:44 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Christophe Leroy,
	Michael Ellerman, Nathan Lynch, Nicholas Piggin, Paul Moore
  Cc: cocci, LKML

Date: Tue, 21 Mar 2023 10:50:08 +0100

The label “out_err” was used to jump to another pointer check despite of
the detail in the implementation of the function “pSeries_reconfig_add_node”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed function call in two cases).

1. Thus return directly after a call of the function “kzalloc” failed.

2. Use more appropriate labels instead.

3. Delete a redundant check.

4. Omit an explicit initialisation for the local variable “err”.

This issue was detected by using the Coccinelle software.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
V2:
This update step was based on a previous change.

 arch/powerpc/platforms/pseries/reconfig.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
index 44f8ebc2ec0d..14154f48ef63 100644
--- a/arch/powerpc/platforms/pseries/reconfig.c
+++ b/arch/powerpc/platforms/pseries/reconfig.c
@@ -23,15 +23,17 @@
 static int pSeries_reconfig_add_node(const char *path, struct property *proplist)
 {
 	struct device_node *np;
-	int err = -ENOMEM;
+	int err;

 	np = kzalloc(sizeof(*np), GFP_KERNEL);
 	if (!np)
-		goto out_err;
+		return -ENOMEM;

 	np->full_name = kstrdup(kbasename(path), GFP_KERNEL);
-	if (!np->full_name)
-		goto out_err;
+	if (!np->full_name) {
+		err = -ENOMEM;
+		goto free_device_node;
+	}

 	np->properties = proplist;
 	of_node_set_flag(np, OF_DYNAMIC);
@@ -46,20 +48,19 @@ static int pSeries_reconfig_add_node(const char *path, struct property *proplist
 	err = of_attach_node(np);
 	if (err) {
 		printk(KERN_ERR "Failed to add device node %s\n", path);
-		goto out_err;
+		goto put_node;
 	}

 	of_node_put(np->parent);

 	return 0;

-out_err:
-	if (np) {
-		of_node_put(np->parent);
+put_node:
+	of_node_put(np->parent);
 free_name:
-		kfree(np->full_name);
-		kfree(np);
-	}
+	kfree(np->full_name);
+free_device_node:
+	kfree(np);
 	return err;
 }

--
2.40.0


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

* [cocci] [PATCH resent] dmaengine: bestcomm: Fix exception handling in bcom_task_alloc()
       [not found] ` <eaa9f90f-c91b-dc87-51a1-d97f99fc5b4b@web.de>
@ 2023-03-25 14:00   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 14:00 UTC (permalink / raw)
  To: kernel-janitors, dmaengine, Christophe Leroy, Grant Likely,
	Greg Kroah-Hartman, Sylvain Munaut, Thomas Gleixner, Vinod Koul
  Cc: cocci, LKML

Date: Sat, 18 Mar 2023 14:55:02 +0100

The label “error” was used to jump to another pointer check despite of
the detail in the implementation of the function “bcom_task_alloc”
that it was determined already that the corresponding variable
contained a null pointer (because of a failed memory allocation).

1. Use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Omit a questionable call of the function “bcom_sram_free”

4. Delete an extra pointer check which became unnecessary
   with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: 2f9ea1bde0d12d8fb5a7bdc7ab6834275d456262 ("[POWERPC] bestcomm: core bestcomm support for Freescale MPC5200")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/bestcomm/bestcomm.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
index eabbcfcaa7cb..7d6a92d34871 100644
--- a/drivers/dma/bestcomm/bestcomm.c
+++ b/drivers/dma/bestcomm/bestcomm.c
@@ -72,7 +72,7 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
 	/* Allocate our structure */
 	tsk = kzalloc(sizeof(struct bcom_task) + priv_size, GFP_KERNEL);
 	if (!tsk)
-		goto error;
+		goto reset_stop;

 	tsk->tasknum = tasknum;
 	if (priv_size)
@@ -81,18 +81,18 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
 	/* Get IRQ of that task */
 	tsk->irq = irq_of_parse_and_map(bcom_eng->ofnode, tsk->tasknum);
 	if (!tsk->irq)
-		goto error;
+		goto free_task;

 	/* Init the BDs, if needed */
 	if (bd_count) {
 		tsk->cookie = kmalloc_array(bd_count, sizeof(void *),
 					    GFP_KERNEL);
 		if (!tsk->cookie)
-			goto error;
+			goto dispose_mapping;

 		tsk->bd = bcom_sram_alloc(bd_count * bd_size, 4, &tsk->bd_pa);
 		if (!tsk->bd)
-			goto error;
+			goto free_cookie;
 		memset_io(tsk->bd, 0x00, bd_count * bd_size);

 		tsk->num_bd = bd_count;
@@ -101,15 +101,13 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)

 	return tsk;

-error:
-	if (tsk) {
-		if (tsk->irq)
-			irq_dispose_mapping(tsk->irq);
-		bcom_sram_free(tsk->bd);
-		kfree(tsk->cookie);
-		kfree(tsk);
-	}
-
+free_cookie:
+	kfree(tsk->cookie);
+dispose_mapping:
+	irq_dispose_mapping(tsk->irq);
+free_task:
+	kfree(tsk);
+reset_stop:
 	bcom_eng->tdt[tasknum].stop = 0;

 	return NULL;
--
2.40.0


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

* [cocci] [PATCH resent] cpufreq: sparc: Fix exception handling in two functions
       [not found] ` <b3cce5b3-2e68-180c-c293-74d4d9d4032c@web.de>
@ 2023-03-25 14:02   ` Markus Elfring
       [not found]     ` <20230403033529.x6n3ihhkypwizq3b@vireshk-i7>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 14:02 UTC (permalink / raw)
  To: kernel-janitors, linux-pm, Rafael J. Wysocki, Viresh Kumar; +Cc: cocci, LKML

Date: Sat, 18 Mar 2023 11:40:11 +0100

The label “err_out” was used to jump to another pointer check despite of
the detail in the implementation of the functions “us2e_freq_init”
and “us3_freq_init” that it was determined already that the corresponding
variable contained a null pointer (because of a failed memory allocation).

1. Use additional labels.

2. Reorder jump targets at the end.

3. Delete an extra pointer check which became unnecessary
   with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/cpufreq/sparc-us2e-cpufreq.c | 12 ++++++------
 drivers/cpufreq/sparc-us3-cpufreq.c  | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/sparc-us2e-cpufreq.c b/drivers/cpufreq/sparc-us2e-cpufreq.c
index 92acbb25abb3..8534d8b1af56 100644
--- a/drivers/cpufreq/sparc-us2e-cpufreq.c
+++ b/drivers/cpufreq/sparc-us2e-cpufreq.c
@@ -324,12 +324,12 @@ static int __init us2e_freq_init(void)
 		ret = -ENOMEM;
 		driver = kzalloc(sizeof(*driver), GFP_KERNEL);
 		if (!driver)
-			goto err_out;
+			goto reset_freq_table;

 		us2e_freq_table = kzalloc((NR_CPUS * sizeof(*us2e_freq_table)),
 			GFP_KERNEL);
 		if (!us2e_freq_table)
-			goto err_out;
+			goto free_driver;

 		driver->init = us2e_freq_cpu_init;
 		driver->verify = cpufreq_generic_frequency_table_verify;
@@ -346,11 +346,11 @@ static int __init us2e_freq_init(void)
 		return 0;

 err_out:
-		if (driver) {
-			kfree(driver);
-			cpufreq_us2e_driver = NULL;
-		}
 		kfree(us2e_freq_table);
+free_driver:
+		kfree(driver);
+		cpufreq_us2e_driver = NULL;
+reset_freq_table:
 		us2e_freq_table = NULL;
 		return ret;
 	}
diff --git a/drivers/cpufreq/sparc-us3-cpufreq.c b/drivers/cpufreq/sparc-us3-cpufreq.c
index e41b35b16afd..325f61bb2e40 100644
--- a/drivers/cpufreq/sparc-us3-cpufreq.c
+++ b/drivers/cpufreq/sparc-us3-cpufreq.c
@@ -172,12 +172,12 @@ static int __init us3_freq_init(void)
 		ret = -ENOMEM;
 		driver = kzalloc(sizeof(*driver), GFP_KERNEL);
 		if (!driver)
-			goto err_out;
+			goto reset_freq_table;

 		us3_freq_table = kzalloc((NR_CPUS * sizeof(*us3_freq_table)),
 			GFP_KERNEL);
 		if (!us3_freq_table)
-			goto err_out;
+			goto free_driver;

 		driver->init = us3_freq_cpu_init;
 		driver->verify = cpufreq_generic_frequency_table_verify;
@@ -194,11 +194,11 @@ static int __init us3_freq_init(void)
 		return 0;

 err_out:
-		if (driver) {
-			kfree(driver);
-			cpufreq_us3_driver = NULL;
-		}
 		kfree(us3_freq_table);
+free_driver:
+		kfree(driver);
+		cpufreq_us3_driver = NULL;
+reset_freq_table:
 		us3_freq_table = NULL;
 		return ret;
 	}
--
2.40.0


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

* [cocci] [PATCH resent] clk: at91: sama7g5: Add two jump labels in sama7g5_pmc_setup()
       [not found] ` <5ed1bc78-77a1-8eb8-43f9-6005d7de49c8@web.de>
@ 2023-03-25 14:05   ` Markus Elfring
       [not found]     ` <7890284f-5809-2f46-9d5f-52e20a3ec327@microchip.com>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 14:05 UTC (permalink / raw)
  To: kernel-janitors, linux-clk, linux-arm-kernel, Alexandre Belloni,
	Claudiu Beznea, Michael Turquette, Nicolas Ferre, Stephen Boyd
  Cc: cocci, LKML

Date: Fri, 17 Mar 2023 20:02:34 +0100

The label “err_free” was used to jump to another pointer check despite of
the detail in the implementation of the function “sama7g5_pmc_setup”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed memory allocation).

* Thus use additional labels.

* Delete an extra pointer check at the end which became unnecessary
  with this refactoring.

This issue was detected by using the Coccinelle software.

Fixes: cb783bbbcf54c36256006895c215e86c5e7266d8 ("clk: at91: sama7g5: add clock support for sama7g5")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/clk/at91/sama7g5.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c
index f135b662f1ff..224b1f2ebef2 100644
--- a/drivers/clk/at91/sama7g5.c
+++ b/drivers/clk/at91/sama7g5.c
@@ -927,25 +927,25 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 			    (ARRAY_SIZE(sama7g5_mckx) + ARRAY_SIZE(sama7g5_gck)),
 			    GFP_KERNEL);
 	if (!alloc_mem)
-		goto err_free;
+		goto free_pmc;

 	hw = at91_clk_register_main_rc_osc(regmap, "main_rc_osc", 12000000,
 					   50000000);
 	if (IS_ERR(hw))
-		goto err_free;
+		goto free_alloc_mem;

 	bypass = of_property_read_bool(np, "atmel,osc-bypass");

 	hw = at91_clk_register_main_osc(regmap, "main_osc", mainxtal_name,
 					bypass);
 	if (IS_ERR(hw))
-		goto err_free;
+		goto free_alloc_mem;

 	parent_names[0] = "main_rc_osc";
 	parent_names[1] = "main_osc";
 	hw = at91_clk_register_sam9x5_main(regmap, "mainck", parent_names, 2);
 	if (IS_ERR(hw))
-		goto err_free;
+		goto free_alloc_mem;

 	sama7g5_pmc->chws[PMC_MAIN] = hw;

@@ -987,7 +987,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 			}

 			if (IS_ERR(hw))
-				goto err_free;
+				goto free_alloc_mem;

 			if (sama7g5_plls[i][j].eid)
 				sama7g5_pmc->chws[sama7g5_plls[i][j].eid] = hw;
@@ -999,7 +999,7 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 					  &mck0_layout, &mck0_characteristics,
 					  &pmc_mck0_lock, CLK_GET_RATE_NOCACHE, 5);
 	if (IS_ERR(hw))
-		goto err_free;
+		goto free_alloc_mem;

 	sama7g5_pmc->chws[PMC_MCK] = hw;

@@ -1128,12 +1128,11 @@ static void __init sama7g5_pmc_setup(struct device_node *np)
 	return;

 err_free:
-	if (alloc_mem) {
-		for (i = 0; i < alloc_mem_size; i++)
-			kfree(alloc_mem[i]);
-		kfree(alloc_mem);
-	}
-
+	for (i = 0; i < alloc_mem_size; i++)
+		kfree(alloc_mem[i]);
+free_alloc_mem:
+	kfree(alloc_mem);
+free_pmc:
 	kfree(sama7g5_pmc);
 }

--
2.40.0


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

* [cocci] [PATCH resent] drbd: Fix exception handling in nla_put_drbd_cfg_context()
       [not found] ` <8d193937-532f-959f-9b84-d911984508aa@web.de>
@ 2023-03-25 14:07   ` Markus Elfring
  2023-03-27 12:28     ` Christoph Böhmwalder
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 14:07 UTC (permalink / raw)
  To: kernel-janitors, drbd-dev, linux-block,
	Christoph Böhmwalder, Jens Axboe, Lars Ellenberg,
	Philipp Reisner
  Cc: cocci, LKML

Date: Fri, 17 Mar 2023 18:32:05 +0100

The label “nla_put_failure” was used to jump to another pointer check
despite of the detail in the implementation of the function
“nla_put_drbd_cfg_context” that it was determined already that
the corresponding variable contained a null pointer.

* Thus return directly after a call of the function
  “nla_nest_start_noflag” failed.

* Delete an extra pointer check which became unnecessary
  with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: 543cc10b4cc5c60aa9fcc62705ccfb9998bf4697 ("drbd: drbd_adm_get_status needs to show some more detail")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/block/drbd/drbd_nl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index f49f2a5282e1..9cb947127472 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -3187,7 +3187,7 @@ static int nla_put_drbd_cfg_context(struct sk_buff *skb,
 	struct nlattr *nla;
 	nla = nla_nest_start_noflag(skb, DRBD_NLA_CFG_CONTEXT);
 	if (!nla)
-		goto nla_put_failure;
+		return -EMSGSIZE;
 	if (device &&
 	    nla_put_u32(skb, T_ctx_volume, device->vnr))
 		goto nla_put_failure;
@@ -3205,8 +3205,7 @@ static int nla_put_drbd_cfg_context(struct sk_buff *skb,
 	return 0;

 nla_put_failure:
-	if (nla)
-		nla_nest_cancel(skb, nla);
+	nla_nest_cancel(skb, nla);
 	return -EMSGSIZE;
 }

--
2.40.0


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

* [cocci] [PATCH resent] blk-mq: Add two labels in blk_rq_prep_clone()
       [not found] ` <3151f1ef-63c6-d016-7c6a-2572e3d93d8f@web.de>
@ 2023-03-25 14:10   ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 14:10 UTC (permalink / raw)
  To: kernel-janitors, linux-block, Jens Axboe, Johannes Thumshirn; +Cc: cocci, LKML

Date: Fri, 17 Mar 2023 17:20:22 +0100

The label “free_and_out” was used to jump to another pointer check despite of
the detail in the implementation of the function “blk_rq_prep_clone”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed function call).

Thus use additional labels so that duplicated checks will be avoided.

This issue was detected by using the Coccinelle software.

Fixes: 06c8c691e2820077936e59ad334eb806e90b69eb ("block: move request based cloning helpers to blk-mq.c")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 block/blk-mq.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a875b1cdff9b..4b290d8cb6a0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3118,10 +3118,10 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
 		bio = bio_alloc_clone(rq->q->disk->part0, bio_src, gfp_mask,
 				      bs);
 		if (!bio)
-			goto free_and_out;
+			goto unprep_clone;

 		if (bio_ctr && bio_ctr(bio, bio_src, data))
-			goto free_and_out;
+			goto put_bio;

 		if (rq->bio) {
 			rq->biotail->bi_next = bio;
@@ -3149,7 +3149,9 @@ int blk_rq_prep_clone(struct request *rq, struct request *rq_src,

 free_and_out:
 	if (bio)
+put_bio:
 		bio_put(bio);
+unprep_clone:
 	blk_rq_unprep_clone(rq);

 	return -ENOMEM;
--
2.40.0


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

* [cocci] [PATCH resent] perf/x86/amd/uncore: Fix exception handling in amd_uncore_cpu_up_prepare()
       [not found] ` <ab860edf-79ca-2035-c5a3-d25be6fd9dac@web.de>
@ 2023-03-25 14:15   ` Markus Elfring
       [not found]     ` <d691d740-c172-a5cb-e4f0-5bc5687c8464@intel.com>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 14:15 UTC (permalink / raw)
  To: kernel-janitors, linux-perf-users, Adrian Hunter,
	Arnaldo Carvalho de Melo, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, Jiri Olsa, Mark Rutland,
	Namhyung Kim, Peter Zijlstra, Sandipan Das, Thomas Gleixner,
	Zhouyi Zhou, x86
  Cc: cocci, LKML

Date: Fri, 17 Mar 2023 13:13:14 +0100

The label “fail” was used to jump to another pointer check despite of
the detail in the implementation of the function “amd_uncore_cpu_up_prepare”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed function call in two cases).

1. Thus return directly after a call of the function “amd_uncore_alloc”
   failed in the first if branch.

2. Use more appropriate labels instead.

3. Reorder jump targets at the end.

4. Delete a redundant check and kfree() call.

5. Omit an explicit initialisation for the local variable “uncore_llc”.


This issue was detected by using the Coccinelle software.

Fixes: 39621c5808f5dda75d03dc4b2d4d2b13a5a1c34b ("perf/x86/amd/uncore: Use dynamic events array")
Fixes: 503d3291a937b726757c1f7c45fa02389d2f4324 ("perf/x86/amd: Try to fix some mem allocation failure handling")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/x86/events/amd/uncore.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index 83f15fe411b3..0a9b5cb97bb4 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -440,13 +440,13 @@ amd_uncore_events_alloc(unsigned int num, unsigned int cpu)

 static int amd_uncore_cpu_up_prepare(unsigned int cpu)
 {
-	struct amd_uncore *uncore_nb = NULL, *uncore_llc = NULL;
+	struct amd_uncore *uncore_nb = NULL, *uncore_llc;

 	if (amd_uncore_nb) {
 		*per_cpu_ptr(amd_uncore_nb, cpu) = NULL;
 		uncore_nb = amd_uncore_alloc(cpu);
 		if (!uncore_nb)
-			goto fail;
+			return -ENOMEM;
 		uncore_nb->cpu = cpu;
 		uncore_nb->num_counters = num_counters_nb;
 		uncore_nb->rdpmc_base = RDPMC_BASE_NB;
@@ -455,7 +455,7 @@ static int amd_uncore_cpu_up_prepare(unsigned int cpu)
 		uncore_nb->pmu = &amd_nb_pmu;
 		uncore_nb->events = amd_uncore_events_alloc(num_counters_nb, cpu);
 		if (!uncore_nb->events)
-			goto fail;
+			goto free_nb;
 		uncore_nb->id = -1;
 		*per_cpu_ptr(amd_uncore_nb, cpu) = uncore_nb;
 	}
@@ -464,7 +464,7 @@ static int amd_uncore_cpu_up_prepare(unsigned int cpu)
 		*per_cpu_ptr(amd_uncore_llc, cpu) = NULL;
 		uncore_llc = amd_uncore_alloc(cpu);
 		if (!uncore_llc)
-			goto fail;
+			goto check_uncore_nb;
 		uncore_llc->cpu = cpu;
 		uncore_llc->num_counters = num_counters_llc;
 		uncore_llc->rdpmc_base = RDPMC_BASE_LLC;
@@ -473,24 +473,22 @@ static int amd_uncore_cpu_up_prepare(unsigned int cpu)
 		uncore_llc->pmu = &amd_llc_pmu;
 		uncore_llc->events = amd_uncore_events_alloc(num_counters_llc, cpu);
 		if (!uncore_llc->events)
-			goto fail;
+			goto free_llc;
 		uncore_llc->id = -1;
 		*per_cpu_ptr(amd_uncore_llc, cpu) = uncore_llc;
 	}

 	return 0;

-fail:
+free_llc:
+	kfree(uncore_llc);
+check_uncore_nb:
 	if (uncore_nb) {
 		kfree(uncore_nb->events);
+free_nb:
 		kfree(uncore_nb);
 	}

-	if (uncore_llc) {
-		kfree(uncore_llc->events);
-		kfree(uncore_llc);
-	}
-
 	return -ENOMEM;
 }

--
2.40.0


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

* [cocci] [PATCH v2 0/4] powerpc/4xx: Adjustments for four function implementations
  2023-03-16 20:07 ` [cocci] [PATCH 0/4] powerpc/4xx: Adjustments for four function implementations Markus Elfring
@ 2023-03-25 15:30   ` Markus Elfring
  2023-03-25 15:36     ` [cocci] [PATCH v2 1/4] powerpc/4xx: Fix exception handling in ppc4xx_pciex_port_setup_hose() Markus Elfring
                       ` (4 more replies)
  0 siblings, 5 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 15:30 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Benjamin Herrenschmidt,
	Christophe Leroy, Josh Boyer, Michael Ellerman, Nicholas Piggin,
	Stefan Roese
  Cc: cocci, LKML

Date: Sat, 25 Mar 2023 16:10:23 +0100

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Fix exception handling in ppc4xx_pciex_port_setup_hose()
  Fix exception handling in ppc4xx_probe_pcix_bridge()
  Fix exception handling in ppc4xx_probe_pci_bridge()
  Delete unnecessary variable initialisations in four functions

 arch/powerpc/platforms/4xx/pci.c | 69 ++++++++++++++------------------
 1 file changed, 31 insertions(+), 38 deletions(-)

--
2.40.0


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

* [cocci] [PATCH v2 1/4] powerpc/4xx: Fix exception handling in ppc4xx_pciex_port_setup_hose()
  2023-03-25 15:30   ` [cocci] [PATCH v2 " Markus Elfring
@ 2023-03-25 15:36     ` Markus Elfring
  2023-03-25 15:38     ` [cocci] [PATCH v2 2/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pcix_bridge() Markus Elfring
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 15:36 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Benjamin Herrenschmidt,
	Christophe Leroy, Josh Boyer, Michael Ellerman, Nicholas Piggin,
	Stefan Roese
  Cc: cocci, LKML

Date: Thu, 16 Mar 2023 19:00:57 +0100

The label “fail” was used to jump to another pointer check despite of
the detail in the implementation of the function “ppc4xx_pciex_port_setup_hose”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed function call in three cases).

1. Thus return directly after a call of the function “pcibios_alloc_controller” failed.

2. Use more appropriate labels instead.

3. Reorder jump targets at the end.

4. Delete two questionable checks.


This issue was detected by using the Coccinelle software.

Fixes: a2d2e1ec07a80946cbe812dc8c73291cad8214b2 ("[POWERPC] 4xx: PLB to PCI Express support")
Fixes: 80daac3f86d4f5aafc9d3e79addb90fa118244e2 ("[POWERPC] 4xx: Add endpoint support to 4xx PCIe driver")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/platforms/4xx/pci.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index ca5dd7a5842a..7336c7039b10 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -1930,7 +1930,7 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	/* Allocate the host controller data structure */
 	hose = pcibios_alloc_controller(port->node);
 	if (!hose)
-		goto fail;
+		return;

 	/* We stick the port number in "indirect_type" so the config space
 	 * ops can retrieve the port data structure easily
@@ -1962,7 +1962,7 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 		if (cfg_data == NULL) {
 			printk(KERN_ERR "%pOF: Can't map external config space !",
 			       port->node);
-			goto fail;
+			goto free_controller;
 		}
 		hose->cfg_data = cfg_data;
 	}
@@ -1974,7 +1974,7 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	if (mbase == NULL) {
 		printk(KERN_ERR "%pOF: Can't map internal config space !",
 		       port->node);
-		goto fail;
+		goto recheck_cfg_data;
 	}
 	hose->cfg_addr = mbase;

@@ -2007,7 +2007,7 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)

 	/* Parse inbound mapping resources */
 	if (ppc4xx_parse_dma_ranges(hose, mbase, &dma_window) != 0)
-		goto fail;
+		goto unmap_io_mbase;

 	/* Configure outbound ranges POMs */
 	ppc4xx_configure_pciex_POMs(port, hose, mbase);
@@ -2064,13 +2064,14 @@ static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 	}

 	return;
- fail:
-	if (hose)
-		pcibios_free_controller(hose);
+
+unmap_io_mbase:
+	iounmap(mbase);
+recheck_cfg_data:
 	if (cfg_data)
 		iounmap(cfg_data);
-	if (mbase)
-		iounmap(mbase);
+free_controller:
+	pcibios_free_controller(hose);
 }

 static void __init ppc4xx_probe_pciex_bridge(struct device_node *np)
--
2.40.0


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

* [cocci] [PATCH v2 2/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pcix_bridge()
  2023-03-25 15:30   ` [cocci] [PATCH v2 " Markus Elfring
  2023-03-25 15:36     ` [cocci] [PATCH v2 1/4] powerpc/4xx: Fix exception handling in ppc4xx_pciex_port_setup_hose() Markus Elfring
@ 2023-03-25 15:38     ` Markus Elfring
  2023-03-25 15:40     ` [cocci] [PATCH v2 3/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pci_bridge() Markus Elfring
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 15:38 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Benjamin Herrenschmidt,
	Christophe Leroy, Josh Boyer, Michael Ellerman, Nicholas Piggin,
	Stefan Roese
  Cc: cocci, LKML

Date: Thu, 16 Mar 2023 19:09:33 +0100

The label “fail” was used to jump to another pointer check despite of
the detail in the implementation of the function “ppc4xx_probe_pcix_bridge”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed function call in two cases).

1. Thus return directly after a call of the function “ioremap” failed.

2. Use a more appropriate label instead.

3. Delete two questionable checks.

4. Adjust the exception handling for another if branch a bit more.

5. Remove a return statement at the end.


This issue was detected by using the Coccinelle software.

Fixes: 5738ec6d00b7abbcd4cd342af83fd18d192b0979 ("[POWERPC] 4xx: PLB to PCI-X support")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/platforms/4xx/pci.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index 7336c7039b10..fdf13e12ca9d 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -564,13 +564,13 @@ static void __init ppc4xx_probe_pcix_bridge(struct device_node *np)
 	reg = ioremap(rsrc_reg.start, resource_size(&rsrc_reg));
 	if (reg == NULL) {
 		printk(KERN_ERR "%pOF: Can't map registers !", np);
-		goto fail;
+		return;
 	}

 	/* Allocate the host controller data structure */
 	hose = pcibios_alloc_controller(np);
 	if (!hose)
-		goto fail;
+		goto unmap_io;

 	hose->first_busno = bus_range ? bus_range[0] : 0x0;
 	hose->last_busno = bus_range ? bus_range[1] : 0xff;
@@ -595,8 +595,10 @@ static void __init ppc4xx_probe_pcix_bridge(struct device_node *np)
 	pci_process_bridge_OF_ranges(hose, np, primary);

 	/* Parse inbound mapping resources */
-	if (ppc4xx_parse_dma_ranges(hose, reg, &dma_window) != 0)
-		goto fail;
+	if (ppc4xx_parse_dma_ranges(hose, reg, &dma_window)) {
+		pcibios_free_controller(hose);
+		goto unmap_io;
+	}

 	/* Configure outbound ranges POMs */
 	ppc4xx_configure_pcix_POMs(hose, reg);
@@ -605,14 +607,8 @@ static void __init ppc4xx_probe_pcix_bridge(struct device_node *np)
 	ppc4xx_configure_pcix_PIMs(hose, reg, &dma_window, big_pim, msi);

 	/* We don't need the registers anymore */
+unmap_io:
 	iounmap(reg);
-	return;
-
- fail:
-	if (hose)
-		pcibios_free_controller(hose);
-	if (reg)
-		iounmap(reg);
 }

 #ifdef CONFIG_PPC4xx_PCI_EXPRESS
--
2.40.0


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

* [cocci] [PATCH v2 3/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pci_bridge()
  2023-03-25 15:30   ` [cocci] [PATCH v2 " Markus Elfring
  2023-03-25 15:36     ` [cocci] [PATCH v2 1/4] powerpc/4xx: Fix exception handling in ppc4xx_pciex_port_setup_hose() Markus Elfring
  2023-03-25 15:38     ` [cocci] [PATCH v2 2/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pcix_bridge() Markus Elfring
@ 2023-03-25 15:40     ` Markus Elfring
  2023-03-25 15:42     ` [cocci] [PATCH v2 4/4] powerpc/4xx: Delete unnecessary variable initialisations in four functions Markus Elfring
  2024-01-05 17:42     ` [cocci] [PATCH v2 0/4] powerpc/4xx: Adjustments for four function implementations Markus Elfring
  4 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 15:40 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Benjamin Herrenschmidt,
	Christophe Leroy, Josh Boyer, Michael Ellerman, Nicholas Piggin,
	Stefan Roese
  Cc: cocci, LKML

Date: Sat, 25 Mar 2023 15:56:09 +0100

The label “fail” was used to jump to another pointer check despite of
the detail in the implementation of the function “ppc4xx_probe_pci_bridge”
that it was determined already that the corresponding variable contained
a null pointer (because of a failed function call in two cases).

1. Thus return directly after a call of the function “ioremap” failed.

2. Use a more appropriate label instead.

3. Delete two questionable checks.

4. Adjust the exception handling for another if branch a bit more.

5. Remove a return statement at the end.


This issue was detected by using the Coccinelle software.

Fixes: c839e0eff500af03de65e560c2e21c3831586e6e ("[POWERPC] 4xx: PLB to PCI 2.x support")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
V3:
A closing parenthesis (which was overlooked somehow) was preserved
for an if statement.

 arch/powerpc/platforms/4xx/pci.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index fdf13e12ca9d..46ba0a4e5b04 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -358,13 +358,13 @@ static void __init ppc4xx_probe_pci_bridge(struct device_node *np)
 	reg = ioremap(rsrc_reg.start, resource_size(&rsrc_reg));
 	if (reg == NULL) {
 		printk(KERN_ERR "%pOF: Can't map registers !", np);
-		goto fail;
+		return;
 	}

 	/* Allocate the host controller data structure */
 	hose = pcibios_alloc_controller(np);
 	if (!hose)
-		goto fail;
+		goto unmap_io;

 	hose->first_busno = bus_range ? bus_range[0] : 0x0;
 	hose->last_busno = bus_range ? bus_range[1] : 0xff;
@@ -383,8 +383,10 @@ static void __init ppc4xx_probe_pci_bridge(struct device_node *np)
 	pci_process_bridge_OF_ranges(hose, np, primary);

 	/* Parse inbound mapping resources */
-	if (ppc4xx_parse_dma_ranges(hose, reg, &dma_window) != 0)
-		goto fail;
+	if (ppc4xx_parse_dma_ranges(hose, reg, &dma_window)) {
+		pcibios_free_controller(hose);
+		goto unmap_io;
+	}

 	/* Configure outbound ranges POMs */
 	ppc4xx_configure_pci_PMMs(hose, reg);
@@ -393,14 +395,8 @@ static void __init ppc4xx_probe_pci_bridge(struct device_node *np)
 	ppc4xx_configure_pci_PTMs(hose, reg, &dma_window);

 	/* We don't need the registers anymore */
+unmap_io:
 	iounmap(reg);
-	return;
-
- fail:
-	if (hose)
-		pcibios_free_controller(hose);
-	if (reg)
-		iounmap(reg);
 }

 /*
--
2.40.0


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

* [cocci] [PATCH v2 4/4] powerpc/4xx: Delete unnecessary variable initialisations in four functions
  2023-03-25 15:30   ` [cocci] [PATCH v2 " Markus Elfring
                       ` (2 preceding siblings ...)
  2023-03-25 15:40     ` [cocci] [PATCH v2 3/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pci_bridge() Markus Elfring
@ 2023-03-25 15:42     ` Markus Elfring
  2024-01-05 17:42     ` [cocci] [PATCH v2 0/4] powerpc/4xx: Adjustments for four function implementations Markus Elfring
  4 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 15:42 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Benjamin Herrenschmidt,
	Christophe Leroy, Josh Boyer, Michael Ellerman, Nicholas Piggin,
	Stefan Roese
  Cc: cocci, LKML

Date: Sat, 25 Mar 2023 16:00:36 +0100

Some local variables will be set to an appropriate value before usage.
Thus omit explicit initialisations at the beginning of these functions.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/powerpc/platforms/4xx/pci.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
index 46ba0a4e5b04..6fb7f9c966a6 100644
--- a/arch/powerpc/platforms/4xx/pci.c
+++ b/arch/powerpc/platforms/4xx/pci.c
@@ -323,8 +323,8 @@ static void __init ppc4xx_probe_pci_bridge(struct device_node *np)
 	struct resource rsrc_cfg;
 	struct resource rsrc_reg;
 	struct resource dma_window;
-	struct pci_controller *hose = NULL;
-	void __iomem *reg = NULL;
+	struct pci_controller *hose;
+	void __iomem *reg;
 	const int *bus_range;
 	int primary = 0;

@@ -523,8 +523,8 @@ static void __init ppc4xx_probe_pcix_bridge(struct device_node *np)
 	struct resource rsrc_cfg;
 	struct resource rsrc_reg;
 	struct resource dma_window;
-	struct pci_controller *hose = NULL;
-	void __iomem *reg = NULL;
+	struct pci_controller *hose;
+	void __iomem *reg;
 	const int *bus_range;
 	int big_pim = 0, msi = 0, primary = 0;

@@ -1403,7 +1403,7 @@ static struct ppc4xx_pciex_hwops ppc_476fpe_pcie_hwops __initdata =
 static int __init ppc4xx_pciex_check_core_init(struct device_node *np)
 {
 	static int core_init;
-	int count = -ENODEV;
+	int count;

 	if (core_init++)
 		return 0;
@@ -1905,10 +1905,10 @@ static void __init ppc4xx_configure_pciex_PIMs(struct ppc4xx_pciex_port *port,
 static void __init ppc4xx_pciex_port_setup_hose(struct ppc4xx_pciex_port *port)
 {
 	struct resource dma_window;
-	struct pci_controller *hose = NULL;
+	struct pci_controller *hose;
 	const int *bus_range;
 	int primary = 0, busses;
-	void __iomem *mbase = NULL, *cfg_data = NULL;
+	void __iomem *mbase, *cfg_data = NULL;
 	const u32 *pval;
 	u32 val;

--
2.40.0


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

* Re: [cocci] bcache: Fix exception handling in mca_alloc()
       [not found]         ` <BE6CEE57-E9AF-4F17-B281-1E00C5DC2A9C@suse.de>
@ 2023-03-25 17:10           ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 17:10 UTC (permalink / raw)
  To: Coly Li, linux-bcache, kernel-janitors; +Cc: Kent Overstreet, cocci, LKML

> Yes the extra ‘if’ check can be avoided,

Thanks for such an acknowledgement.


> but the code is more simple before adding labels unlock and cannibalize_mca.

I became curious how views will evolve further according to affected
coding style aspects.
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources

Regards,
Markus

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

* [cocci] [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (27 preceding siblings ...)
       [not found] ` <ab860edf-79ca-2035-c5a3-d25be6fd9dac@web.de>
@ 2023-03-25 18:30 ` Markus Elfring
       [not found]   ` <fffcd98a-bb73-41cd-8545-0f2c55dd38f9@lucifer.local>
  2023-03-26  8:20 ` [cocci] [PATCH] selinux: Improve exception handling in security_get_bools() Markus Elfring
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
  30 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-25 18:30 UTC (permalink / raw)
  To: kernel-janitors, linux-kselftest, cgroups, linux-mm, Jay Kamat,
	Johannes Weiner, Michal Hocko, Muchun Song, Roman Gushchin,
	Shakeel Butt, Shuah Khan, Tejun Heo, Zefan Li
  Cc: cocci, LKML

Date: Sat, 25 Mar 2023 19:11:13 +0100

The label “cleanup” was used to jump to another pointer check despite of
the detail in the implementation of the function
“test_memcg_oom_group_score_events” that it was determined already
that a corresponding variable contained a null pointer.

1. Thus return directly after a call of the function “cg_name” failed.

2. Use an additional label.

3. Delete a questionable check.


This issue was detected by using the Coccinelle software.

Fixes: a987785dcd6c8ae2915460582aebd6481c81eb67 ("Add tests for memory.oom.group")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 tools/testing/selftests/cgroup/test_memcontrol.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index f4f7c0aef702..afcd1752413e 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -1242,12 +1242,11 @@ static int test_memcg_oom_group_score_events(const char *root)
 	int safe_pid;

 	memcg = cg_name(root, "memcg_test_0");
-
 	if (!memcg)
-		goto cleanup;
+		return ret;

 	if (cg_create(memcg))
-		goto cleanup;
+		goto free_cg;

 	if (cg_write(memcg, "memory.max", "50M"))
 		goto cleanup;
@@ -1275,8 +1274,8 @@ static int test_memcg_oom_group_score_events(const char *root)
 	ret = KSFT_PASS;

 cleanup:
-	if (memcg)
-		cg_destroy(memcg);
+	cg_destroy(memcg);
+free_cg:
 	free(memcg);

 	return ret;
--
2.40.0


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

* Re: [cocci] [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events()
       [not found]   ` <fffcd98a-bb73-41cd-8545-0f2c55dd38f9@lucifer.local>
@ 2023-03-26  8:15     ` Markus Elfring
       [not found]       ` <20230326213900.GJ363182@maniforge>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-26  8:15 UTC (permalink / raw)
  To: Lorenzo Stoakes, kernel-janitors, linux-kselftest, cgroups,
	linux-mm, Jay Kamat, Johannes Weiner, Michal Hocko, Muchun Song,
	Roman Gushchin, Shakeel Butt, Shuah Khan, Tejun Heo, Zefan Li
  Cc: cocci, LKML

>> The label “cleanup” was used to jump to another pointer check despite of
>> the detail in the implementation of the function
>> “test_memcg_oom_group_score_events” that it was determined already
>> that a corresponding variable contained a null pointer.
>
> This is poorly writte and confusing.

A special source code combination was detected.


Reconsidering repeated pointer checks with SmPL
https://lore.kernel.org/cocci/f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2023-03/msg00017.html



> Something like 'avoid unnecessary null check/cg_destroy() invocation'
> would be far clearer.

I (or another interested contributor) can integrate such information into
a subsequent patch if the change acceptance will grow accordingly.


>> 1. Thus return directly after a call of the function “cg_name” failed.
>
> This feels superfluious.

Under which circumstances would you care more for the advice
“If there is no cleanup needed then just return directly.”
from the section “Centralized exiting of functions”?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v6.3-rc3#n532


>> 2. Use an additional label.
>
> This also feels superfluious.

The marking of special source code positions is probably required for
the presented design goal.


>> 3. Delete a questionable check.
>
> This seems superfluois and frankly, rude. It's not questionable,

Can you find the combination of code fragments like “if (!memcg)”
and “if (memcg)” suspicious?


> it's readable, you should try to avoid language like 'questionable' when the
> purpose of the check is obvious.

I tend to prefer an other known design variant at this place.


>> This issue was detected by using the Coccinelle software.
>>
>> Fixes: a987785dcd6c8ae2915460582aebd6481c81eb67 ("Add tests for memory.oom.group")
>
> Fixes what in the what now?

1. Check repetition (which can be undesirable)

2. Can a cg_destroy() call ever work as expected if a cg_create() call failed?


>> +++ b/tools/testing/selftests/cgroup/test_memcontrol.c
>> @@ -1242,12 +1242,11 @@ static int test_memcg_oom_group_score_events(const char *root)
>>  	int safe_pid;
>>
>>  	memcg = cg_name(root, "memcg_test_0");
>> -
>>  	if (!memcg)
>> -		goto cleanup;
>> +		return ret;
>>
>>  	if (cg_create(memcg))
>> -		goto cleanup;
>> +		goto free_cg;
>>
>>  	if (cg_write(memcg, "memory.max", "50M"))
>>  		goto cleanup;
>> @@ -1275,8 +1274,8 @@ static int test_memcg_oom_group_score_events(const char *root)
>>  	ret = KSFT_PASS;
>>
>>  cleanup:
>> -	if (memcg)
>> -		cg_destroy(memcg);
>> +	cg_destroy(memcg);
>> +free_cg:
>>  	free(memcg);
>>
>>  	return ret;
>> --
> I dislike this patch, it adds complexity for no discernible purpose and
> actively makes the code _less_ readable and in a self-test of all places (!)

It seems that there is a conflict to resolve also according to another
information source.
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28POSIX,GotoChain%29


Regards,
Markus

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

* [cocci] [PATCH] selinux: Improve exception handling in security_get_bools()
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (28 preceding siblings ...)
  2023-03-25 18:30 ` [cocci] [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events() Markus Elfring
@ 2023-03-26  8:20 ` Markus Elfring
       [not found]   ` <tencent_1C472DFEAAEC903262297C9B9C978B365909@qq.com>
  2023-03-27  7:00   ` [cocci] [PATCH v2] selinux: Adjust implementation of security_get_bools() Markus Elfring
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
  30 siblings, 2 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-26  8:20 UTC (permalink / raw)
  To: kernel-janitors, selinux, Christian Göttsche, Eric Paris,
	Michal Orzel, Ondrej Mosnacek, Paul Moore, Ruiqi Gong,
	Stephen Smalley, Xiu Jianfeng
  Cc: cocci, LKML

Date: Sat, 25 Mar 2023 22:30:53 +0100

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “security_get_bools”
that it was determined already that a corresponding variable contained
a null pointer because of a failed memory allocation.

1. Thus use more appropriate labels instead.

2. Move the statement “rc = -ENOMEM;” into three if branches.

3. Reorder the assignment targets for two kcalloc() calls.

4. Reorder jump targets at the end.

5. Convert the statement “rc = 0;” into a variable initialisation.

6. Delete an extra pointer check which became unnecessary
   with this refactoring.


This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 security/selinux/ss/services.c | 47 +++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index f14d1ffe54c5..534bf977e95a 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2966,50 +2966,49 @@ int security_get_bools(struct selinux_policy *policy,
 {
 	struct policydb *policydb;
 	u32 i;
-	int rc;
+	int rc = 0;

 	policydb = &policy->policydb;

 	*names = NULL;
 	*values = NULL;
-
-	rc = 0;
 	*len = policydb->p_bools.nprim;
 	if (!*len)
 		goto out;

-	rc = -ENOMEM;
-	*names = kcalloc(*len, sizeof(char *), GFP_ATOMIC);
-	if (!*names)
-		goto err;
-
-	rc = -ENOMEM;
 	*values = kcalloc(*len, sizeof(int), GFP_ATOMIC);
-	if (!*values)
-		goto err;
+	if (!*values) {
+		rc = -ENOMEM;
+		goto reset_len;
+	}
+
+	*names = kcalloc(*len, sizeof(char *), GFP_ATOMIC);
+	if (!*names) {
+		rc = -ENOMEM;
+		goto free_values;
+	}

 	for (i = 0; i < *len; i++) {
 		(*values)[i] = policydb->bool_val_to_struct[i]->state;
-
-		rc = -ENOMEM;
 		(*names)[i] = kstrdup(sym_name(policydb, SYM_BOOLS, i),
 				      GFP_ATOMIC);
-		if (!(*names)[i])
-			goto err;
+		if (!(*names)[i]) {
+			rc = -ENOMEM;
+			goto free_names;
+		}
 	}
-	rc = 0;
 out:
 	return rc;
-err:
-	if (*names) {
-		for (i = 0; i < *len; i++)
-			kfree((*names)[i]);
-		kfree(*names);
-	}
-	kfree(*values);
-	*len = 0;
+free_names:
+	for (i = 0; i < *len; i++)
+		kfree((*names)[i]);
+	kfree(*names);
 	*names = NULL;
+free_values:
+	kfree(*values);
 	*values = NULL;
+reset_len:
+	*len = 0;
 	goto out;
 }

--
2.40.0


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

* Re: [cocci] selinux: Improve exception handling in security_get_bools()
       [not found]   ` <tencent_1C472DFEAAEC903262297C9B9C978B365909@qq.com>
@ 2023-03-27  5:25     ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-27  5:25 UTC (permalink / raw)
  To: Ruiqi Gong, kernel-janitors, selinux, Christian Göttsche,
	Eric Paris, Michal Orzel, Ondrej Mosnacek, Paul Moore,
	Stephen Smalley, Xiu Jianfeng
  Cc: cocci, LKML, Ruiqi Gong

> So I don't see the necessity of splitting the error handling here into
> different parts.

I generally prefer to adapt the exception handling better to a detected failure.

See also:
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28POSIX,GotoChain%29

Regards,
Markus

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

* Re: [cocci] [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events()
       [not found]       ` <20230326213900.GJ363182@maniforge>
@ 2023-03-27  5:56         ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-27  5:56 UTC (permalink / raw)
  To: David Vernet, kernel-janitors, linux-kselftest, cgroups,
	linux-mm, Jay Kamat, Johannes Weiner, Michal Hocko, Muchun Song,
	Roman Gushchin, Shakeel Butt, Shuah Khan, Tejun Heo, Zefan Li,
	Lorenzo Stoakes
  Cc: cocci, LKML

>> 2. Can a cg_destroy() call ever work as expected if a cg_create() call failed?
>
> Perhaps next time you can answer your own question by spending 30
> seconds actually reading the code you're "fixing":
>
> int cg_destroy(const char *cgroup)
> {
>         ret = rmdir(cgroup);
>         if (ret && errno == ENOENT) <<< that case is explicitly handled here
>                 ret = 0;
>
>         return ret;
> }

Is it interesting somehow that a non-existing directory (which would occasionally
not be found) is tolerated so far?
https://elixir.bootlin.com/linux/v6.3-rc3/source/tools/testing/selftests/cgroup/cgroup_util.c#L285

Should such a function call be avoided because of a failed cg_create() call?

Regards,
Markus

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

* [cocci] [PATCH v2] selinux: Adjust implementation of security_get_bools()
  2023-03-26  8:20 ` [cocci] [PATCH] selinux: Improve exception handling in security_get_bools() Markus Elfring
       [not found]   ` <tencent_1C472DFEAAEC903262297C9B9C978B365909@qq.com>
@ 2023-03-27  7:00   ` Markus Elfring
       [not found]     ` <CAHC9VhR=yK72JXW3hJR+gUQtGCNpF0Bzk5RDzPZR0MunC84AUQ@mail.gmail.com>
  1 sibling, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-27  7:00 UTC (permalink / raw)
  To: kernel-janitors, selinux, Christian Göttsche, Eric Paris,
	Michal Orzel, Ondrej Mosnacek, Paul Moore, Ruiqi Gong,
	Stephen Smalley, Xiu Jianfeng
  Cc: cocci, LKML, Ruiqi Gong

Date: Mon, 27 Mar 2023 08:50:56 +0200

The label “err” was used to jump to another pointer check despite of
the detail in the implementation of the function “security_get_bools”
that it was determined already that a corresponding variable contained
a null pointer because of a failed memory allocation.

Thus perform the following adjustments:

1. Convert the statement “policydb = &policy->policydb;” into
   a variable initialisation.

2. Replace the statement “goto out;” by “return -ENOMEM;”.

3. Return zero directly at two places.

4. Omit the variable “rc”.

5. Use more appropriate labels instead.

6. Reorder the assignment targets for two kcalloc() calls.

7. Reorder jump targets at the end.

8. Assign a value element only after a name assignment succeeded.

9. Delete an extra pointer check which became unnecessary
   with this refactoring.


This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 security/selinux/ss/services.c | 52 ++++++++++++++--------------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index f14d1ffe54c5..702282954bf9 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -2964,53 +2964,45 @@ int security_fs_use(struct super_block *sb)
 int security_get_bools(struct selinux_policy *policy,
 		       u32 *len, char ***names, int **values)
 {
-	struct policydb *policydb;
+	struct policydb *policydb = &policy->policydb;
 	u32 i;
-	int rc;
-
-	policydb = &policy->policydb;

 	*names = NULL;
 	*values = NULL;
-
-	rc = 0;
 	*len = policydb->p_bools.nprim;
 	if (!*len)
-		goto out;
-
-	rc = -ENOMEM;
-	*names = kcalloc(*len, sizeof(char *), GFP_ATOMIC);
-	if (!*names)
-		goto err;
+		return 0;

-	rc = -ENOMEM;
 	*values = kcalloc(*len, sizeof(int), GFP_ATOMIC);
 	if (!*values)
-		goto err;
+		goto reset_len;

-	for (i = 0; i < *len; i++) {
-		(*values)[i] = policydb->bool_val_to_struct[i]->state;
+	*names = kcalloc(*len, sizeof(char *), GFP_ATOMIC);
+	if (!*names)
+		goto free_values;

-		rc = -ENOMEM;
+	for (i = 0; i < *len; i++) {
 		(*names)[i] = kstrdup(sym_name(policydb, SYM_BOOLS, i),
 				      GFP_ATOMIC);
 		if (!(*names)[i])
-			goto err;
-	}
-	rc = 0;
-out:
-	return rc;
-err:
-	if (*names) {
-		for (i = 0; i < *len; i++)
-			kfree((*names)[i]);
-		kfree(*names);
+			goto free_names;
+
+		(*values)[i] = policydb->bool_val_to_struct[i]->state;
 	}
-	kfree(*values);
-	*len = 0;
+	return 0;
+
+free_names:
+	for (i = 0; i < *len; i++)
+		kfree((*names)[i]);
+
+	kfree(*names);
 	*names = NULL;
+free_values:
+	kfree(*values);
 	*values = NULL;
-	goto out;
+reset_len:
+	*len = 0;
+	return -ENOMEM;
 }


--
2.40.0


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

* Re: [cocci] [PATCH resent] drbd: Fix exception handling in nla_put_drbd_cfg_context()
  2023-03-25 14:07   ` [cocci] [PATCH resent] drbd: Fix exception handling in nla_put_drbd_cfg_context() Markus Elfring
@ 2023-03-27 12:28     ` Christoph Böhmwalder
  2023-03-27 17:26       ` [cocci] " Markus Elfring
  0 siblings, 1 reply; 99+ messages in thread
From: Christoph Böhmwalder @ 2023-03-27 12:28 UTC (permalink / raw)
  To: Markus Elfring
  Cc: cocci, LKML, kernel-janitors, drbd-dev, linux-block, Jens Axboe,
	Lars Ellenberg, Philipp Reisner

Am 25.03.23 um 15:07 schrieb Markus Elfring:
> Date: Fri, 17 Mar 2023 18:32:05 +0100
> 
> The label “nla_put_failure” was used to jump to another pointer check
> despite of the detail in the implementation of the function
> “nla_put_drbd_cfg_context” that it was determined already that
> the corresponding variable contained a null pointer.
> 
> * Thus return directly after a call of the function
>   “nla_nest_start_noflag” failed.
> 
> * Delete an extra pointer check which became unnecessary
>   with this refactoring.
> 
> 
> This issue was detected by using the Coccinelle software.
> 
> Fixes: 543cc10b4cc5c60aa9fcc62705ccfb9998bf4697 ("drbd: drbd_adm_get_status needs to show some more detail")
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/block/drbd/drbd_nl.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
> index f49f2a5282e1..9cb947127472 100644
> --- a/drivers/block/drbd/drbd_nl.c
> +++ b/drivers/block/drbd/drbd_nl.c
> @@ -3187,7 +3187,7 @@ static int nla_put_drbd_cfg_context(struct sk_buff *skb,
>  	struct nlattr *nla;
>  	nla = nla_nest_start_noflag(skb, DRBD_NLA_CFG_CONTEXT);
>  	if (!nla)
> -		goto nla_put_failure;
> +		return -EMSGSIZE;
>  	if (device &&
>  	    nla_put_u32(skb, T_ctx_volume, device->vnr))
>  		goto nla_put_failure;
> @@ -3205,8 +3205,7 @@ static int nla_put_drbd_cfg_context(struct sk_buff *skb,
>  	return 0;
> 
>  nla_put_failure:
> -	if (nla)
> -		nla_nest_cancel(skb, nla);
> +	nla_nest_cancel(skb, nla);
>  	return -EMSGSIZE;
>  }
> 
> --
> 2.40.0
> 

Sorry, I fail to see how this is an improvement over the status quo,
much less a "fix".

Can you identify the issue with the current code and can you explain how
your patch makes it better?

-- 
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage

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

* Re: [cocci] drbd: Fix exception handling in nla_put_drbd_cfg_context()
  2023-03-27 12:28     ` Christoph Böhmwalder
@ 2023-03-27 17:26       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-27 17:26 UTC (permalink / raw)
  To: Christoph Böhmwalder, kernel-janitors, drbd-dev,
	linux-block, Jens Axboe, Lars Ellenberg, Philipp Reisner
  Cc: cocci, LKML

> Can you identify the issue with the current code and can you explain how
> your patch makes it better?

I propose to avoid a duplicate pointer check also in this function implementation.

Regards,
Markus


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

* Re: [cocci] perf/x86/amd/uncore: Fix exception handling in amd_uncore_cpu_up_prepare()
       [not found]     ` <d691d740-c172-a5cb-e4f0-5bc5687c8464@intel.com>
@ 2023-03-27 17:36       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-27 17:36 UTC (permalink / raw)
  To: Adrian Hunter, kernel-janitors, linux-perf-users,
	Arnaldo Carvalho de Melo, Borislav Petkov, Dave Hansen,
	H. Peter Anvin, Ian Rogers, Ingo Molnar, Jiri Olsa, Mark Rutland,
	Namhyung Kim, Peter Zijlstra, Sandipan Das, Thomas Gleixner,
	Zhouyi Zhou, x86
  Cc: cocci, LKML

> Redundant calls to kfree do not break anything.

Can such function calls influence computation resources in undesirable ways?


> Also avoid using the term "exception" since, in x86, exceptions are
> hardware events.  Better to just call it "error handling".

Will another bit of fine-tuning become helpful for the affected function implementation?

Regards,
Markus

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

* Re: [cocci] selinux: Adjust implementation of security_get_bools()
       [not found]       ` <CAHC9VhREfdgiCji=uEeCrc4w1kPGfnWGKnJuUYKXwTApdneSjQ@mail.gmail.com>
@ 2023-03-28  7:30         ` Markus Elfring
       [not found]           ` <CAHC9VhQfiNd_4uWBmKCC81UnOJb7Y=UFCDMXuqz3=UPr8QtqNw@mail.gmail.com>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-03-28  7:30 UTC (permalink / raw)
  To: Paul Moore, kernel-janitors, selinux, Christian Göttsche,
	Eric Paris, Michal Orzel, Ondrej Mosnacek, Ruiqi Gong,
	Stephen Smalley, Xiu Jianfeng
  Cc: cocci, LKML, Ruiqi Gong

…
>>>  security/selinux/ss/services.c | 52 ++++++++++++++--------------------
> Given the fairly extensive refactoring here,
> If nothing else it will make the function easier to read,
> and I think it will simplify the code a bit too.

I am curious which change possibilities will finally be picked up.


> I would probably also keep the combined @names/@values cleanup under
> one jump label; this function isn't complicated enough to warrant that
> many jump labels for error conditions.

I got an other impression for the affected function implementation.

Would you like to take advice from another information source
better into account?
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28POSIX,GotoChain%29

Regards,
Markus

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

* Re: [cocci] [PATCH resent] clk: at91: sama7g5: Add two jump labels in sama7g5_pmc_setup()
       [not found]     ` <7890284f-5809-2f46-9d5f-52e20a3ec327@microchip.com>
@ 2023-03-28 19:24       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-28 19:24 UTC (permalink / raw)
  To: Nicolas Ferre, kernel-janitors, linux-clk, linux-arm-kernel,
	Alexandre Belloni, Claudiu Beznea, Michael Turquette,
	Stephen Boyd
  Cc: cocci, LKML

>> The label “err_free” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “sama7g5_pmc_setup”
>> that it was determined already that the corresponding variable contained
>> a null pointer (because of a failed memory allocation).
>>
>> * Thus use additional labels.
>>
>> * Delete an extra pointer check at the end which became unnecessary
>>    with this refactoring.
>>
>> This issue was detected by using the Coccinelle software.
>
> Fine, but I'm sorry that it complexity the function for no real value.

Under which circumstances can advice be taken better into account
also from another information source?
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28POSIX,GotoChain%29



> Other clk drivers have the same pattern so I want them to all stay the same.
> This is a NACK, sorry about that.

I am curious if other contributors (or code reviewers) would like to influence
the software situation a bit more.

Regards,
Markus

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

* Re: [cocci] selinux: Adjust implementation of security_get_bools()
       [not found]           ` <CAHC9VhQfiNd_4uWBmKCC81UnOJb7Y=UFCDMXuqz3=UPr8QtqNw@mail.gmail.com>
@ 2023-03-29  5:20             ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-29  5:20 UTC (permalink / raw)
  To: Paul Moore, kernel-janitors, selinux, Christian Göttsche,
	Eric Paris, Michal Orzel, Ondrej Mosnacek, Ruiqi Gong,
	Stephen Smalley, Xiu Jianfeng
  Cc: cocci, LKML, Ruiqi Gong

>> Would you like to take advice from another information source
>> better into account?
>
> In this case, I prefer what I suggested.

What does hinder you to work with more jump labels for improved exception handling?

Regards,
Markus

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

* Re: [cocci] [0/2] md/raid: Adjustments for two function implementations
       [not found]     ` <CAPhsuW7JZDps_fTHyCabjfG4YjzDVEW_41u6d+9mdc2CAJv_Kw@mail.gmail.com>
@ 2023-03-29  5:32       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-03-29  5:32 UTC (permalink / raw)
  To: Song Liu, kernel-janitors, linux-raid, Coly Li, Jens Axboe,
	Kent Overstreet, Maciej Trela, Neil Brown, Shaohua Li
  Cc: cocci, LKML

>>   raid1: Fix exception handling in setup_conf()
>>   raid10: Fix exception handling in setup_conf()
>
> The two functions look good to me as-is. I don't think anything is
> broken except that the code analysis tool complains. I don't think
> it is necessary to make these changes.

Will development interests ever grow also for advice from another information source?
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution%28POSIX,GotoChain%29

Regards,
Markus

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

* Re: [cocci] [PATCH] cpufreq: sparc: Fix exception handling in two functions
       [not found]     ` <20230403033529.x6n3ihhkypwizq3b@vireshk-i7>
@ 2023-04-03 12:50       ` Markus Elfring
       [not found]         ` <20230403230432.xeubpa3cc2gt4mw3@vireshk-i7>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-04-03 12:50 UTC (permalink / raw)
  To: Viresh Kumar, linux-pm, kernel-janitors; +Cc: Rafael J. Wysocki, cocci, LKML

>> +++ b/drivers/cpufreq/sparc-us2e-cpufreq.c
>> @@ -324,12 +324,12 @@ static int __init us2e_freq_init(void)
>>  		ret = -ENOMEM;
>>  		driver = kzalloc(sizeof(*driver), GFP_KERNEL);
>>  		if (!driver)
>> -			goto err_out;
>> +			goto reset_freq_table;
>
> I would just return error from here.

I got the impression from this function implementation that a bit of additional
resource release would be relevant so far (at the end of a corresponding if branch).
https://elixir.bootlin.com/linux/v6.3-rc5/source/drivers/cpufreq/sparc-us2e-cpufreq.c#L309
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpufreq/sparc-us2e-cpufreq.c?h=v6.3-rc5#n309


>>  		us2e_freq_table = kzalloc((NR_CPUS * sizeof(*us2e_freq_table)),
>>  			GFP_KERNEL);
>>  		if (!us2e_freq_table)
>> -			goto err_out;
>> +			goto free_driver;
>>
>>  		driver->init = us2e_freq_cpu_init;
>>  		driver->verify = cpufreq_generic_frequency_table_verify;
>> @@ -346,11 +346,11 @@ static int __init us2e_freq_init(void)
>>  		return 0;
>>
>>  err_out:
>> -		if (driver) {
>> -			kfree(driver);
>> -			cpufreq_us2e_driver = NULL;
>> -		}
>>  		kfree(us2e_freq_table);
>> +free_driver:
>> +		kfree(driver);
>> +		cpufreq_us2e_driver = NULL;
>> +reset_freq_table:
>>  		us2e_freq_table = NULL;
>
> This wasn't set at this point, no point clearing it here. Also this
> clearing of global variables isn't required at all, as after this
> point no other function shall get called.

Do you see further opportunities for refactoring the source code?

Regards,
Markus

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

* [cocci] Reconsidering repeated checks for error pointers (with SmPL)
  2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
                   ` (29 preceding siblings ...)
  2023-03-26  8:20 ` [cocci] [PATCH] selinux: Improve exception handling in security_get_bools() Markus Elfring
@ 2023-04-04  8:00 ` Markus Elfring
  2023-04-05 17:10   ` [cocci] [PATCH] drm/nouveau: Add a jump label in nouveau_gem_ioctl_pushbuf() Markus Elfring
                     ` (5 more replies)
  30 siblings, 6 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-04  8:00 UTC (permalink / raw)
  To: cocci, kernel-janitors

Hello,

I tried the following SmPL script out also on the source files from
the software “Linux next-20230404”.

@display@
expression call1, call2, storage;
identifier target;
@@
*storage = call1(...);
 if (IS_ERR(storage))
 {
 ...
    goto target;
 }
 ...
*target:
 if (!IS_ERR(storage))
 {
 ...
*call2(storage);
 ...
 }


7 source files were found where a check was performed for a failed function call
and an opposite check is immediately performed for the same variable.
I guess that pointer checks are repeated at these source code places.
I imagine that such code should be reconsidered once more and improved accordingly.

How do you think about to achieve any more adjustments in this design direction?

Regards,
Markus

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

* [cocci] [PATCH] drm/nouveau: Add a jump label in nouveau_gem_ioctl_pushbuf()
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
@ 2023-04-05 17:10   ` Markus Elfring
  2023-04-05 20:10   ` [cocci] [PATCH] firmware: ti_sci: Fix exception handling in ti_sci_probe() Markus Elfring
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-05 17:10 UTC (permalink / raw)
  To: kernel-janitors, nouveau, dri-devel, Ben Skeggs, Daniel Vetter,
	David Airlie, Karol Herbst, Lyude Paul
  Cc: cocci, LKML

Date: Wed, 5 Apr 2023 18:38:54 +0200

The label “out_prevalid” was used to jump to another pointer check
despite of the detail in the implementation of the function
“nouveau_gem_ioctl_pushbuf” that it was determined already in one case
that the corresponding variable contained an error pointer
because of a failed call of the function “u_memcpya”.

Thus use an additional label.

This issue was detected by using the Coccinelle software.

Fixes: 2be65641642ef423f82162c3a5f28c754d1637d2 ("drm/nouveau: fix relocations applying logic and a double-free")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/gpu/drm/nouveau/nouveau_gem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index f77e44958037..d87e1cb2c933 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -814,7 +814,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
 			reloc = u_memcpya(req->relocs, req->nr_relocs, sizeof(*reloc));
 			if (IS_ERR(reloc)) {
 				ret = PTR_ERR(reloc);
-				goto out_prevalid;
+				goto out_free_bo;
 			}

 			goto revalidate;
@@ -929,6 +929,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
 out_prevalid:
 	if (!IS_ERR(reloc))
 		u_free(reloc);
+out_free_bo:
 	u_free(bo);
 	u_free(push);

--
2.40.0


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

* [cocci] [PATCH] firmware: ti_sci: Fix exception handling in ti_sci_probe()
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
  2023-04-05 17:10   ` [cocci] [PATCH] drm/nouveau: Add a jump label in nouveau_gem_ioctl_pushbuf() Markus Elfring
@ 2023-04-05 20:10   ` Markus Elfring
       [not found]     ` <20230516152043.bq2hnessstrhbc6s@distort>
  2023-04-06 16:10   ` [cocci] [PATCH 0/2] IB/uverbs: Adjustments for create_qp() Markus Elfring
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-04-05 20:10 UTC (permalink / raw)
  To: kernel-janitors, linux-arm-kernel, Nishanth Menon,
	Santosh Shilimkar, Tero Kristo
  Cc: cocci, LKML

Date: Wed, 5 Apr 2023 22:00:18 +0200

The label “out” was used to jump to another pointer check despite of
the detail in the implementation of the function “ti_sci_probe”
that it was determined already that the corresponding variable
contained an error pointer because of a failed call of
the function “mbox_request_channel_byname”.

* Thus use more appropriate labels instead.

* Delete two redundant checks.


This issue was detected by using the Coccinelle software.

Fixes: aa276781a64a5f15ecc21e920960c5b1f84e5fee ("firmware: Add basic support for TI System Control Interface (TI-SCI) protocol")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/firmware/ti_sci.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 039d92a595ec..77012d2f4160 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -3433,18 +3433,18 @@ static int ti_sci_probe(struct platform_device *pdev)
 	info->chan_rx = mbox_request_channel_byname(cl, "rx");
 	if (IS_ERR(info->chan_rx)) {
 		ret = PTR_ERR(info->chan_rx);
-		goto out;
+		goto remove_debugfs;
 	}

 	info->chan_tx = mbox_request_channel_byname(cl, "tx");
 	if (IS_ERR(info->chan_tx)) {
 		ret = PTR_ERR(info->chan_tx);
-		goto out;
+		goto free_mbox_channel_rx;
 	}
 	ret = ti_sci_cmd_get_revision(info);
 	if (ret) {
 		dev_err(dev, "Unable to communicate with TISCI(%d)\n", ret);
-		goto out;
+		goto free_mbox_channel_tx;
 	}

 	ti_sci_setup_ops(info);
@@ -3456,7 +3456,7 @@ static int ti_sci_probe(struct platform_device *pdev)
 		ret = register_restart_handler(&info->nb);
 		if (ret) {
 			dev_err(dev, "reboot registration fail(%d)\n", ret);
-			goto out;
+			goto free_mbox_channel_tx;
 		}
 	}

@@ -3470,11 +3470,12 @@ static int ti_sci_probe(struct platform_device *pdev)
 	mutex_unlock(&ti_sci_list_mutex);

 	return of_platform_populate(dev->of_node, NULL, NULL, dev);
-out:
-	if (!IS_ERR(info->chan_tx))
-		mbox_free_channel(info->chan_tx);
-	if (!IS_ERR(info->chan_rx))
-		mbox_free_channel(info->chan_rx);
+
+free_mbox_channel_tx:
+	mbox_free_channel(info->chan_tx);
+free_mbox_channel_rx:
+	mbox_free_channel(info->chan_rx);
+remove_debugfs:
 	debugfs_remove(info->d);
 	return ret;
 }
--
2.40.0


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

* [cocci] [PATCH 0/2] IB/uverbs: Adjustments for create_qp()
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
  2023-04-05 17:10   ` [cocci] [PATCH] drm/nouveau: Add a jump label in nouveau_gem_ioctl_pushbuf() Markus Elfring
  2023-04-05 20:10   ` [cocci] [PATCH] firmware: ti_sci: Fix exception handling in ti_sci_probe() Markus Elfring
@ 2023-04-06 16:10   ` Markus Elfring
  2023-04-06 16:14     ` [cocci] [PATCH 1/2] IB/uverbs: Improve exception handling in create_qp() Markus Elfring
  2023-04-06 16:18     ` [cocci] [PATCH 2/2] IB/uverbs: Delete a duplicate check " Markus Elfring
  2023-04-06 20:12   ` [cocci] [PATCH] remoteproc: imx_dsp_rproc: Improve exception handling in imx_dsp_rproc_mbox_alloc() Markus Elfring
                     ` (2 subsequent siblings)
  5 siblings, 2 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-06 16:10 UTC (permalink / raw)
  To: kernel-janitors, linux-rdma, Daisuke Matsuda, Doug Ledford,
	Jason Gunthorpe, Leon Romanovsky, Matan Barak, Max Gurtovoy,
	Sagi Grimberg, Yishai Hadas
  Cc: cocci, LKML

Date: Thu, 6 Apr 2023 17:50:05 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Improve exception handling
  Delete a duplicate check

 drivers/infiniband/core/uverbs_cmd.c | 64 +++++++++++++++++++---------
 1 file changed, 43 insertions(+), 21 deletions(-)

--
2.40.0


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

* [cocci] [PATCH 1/2] IB/uverbs: Improve exception handling in create_qp()
  2023-04-06 16:10   ` [cocci] [PATCH 0/2] IB/uverbs: Adjustments for create_qp() Markus Elfring
@ 2023-04-06 16:14     ` Markus Elfring
  2023-04-06 16:18     ` [cocci] [PATCH 2/2] IB/uverbs: Delete a duplicate check " Markus Elfring
  1 sibling, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-06 16:14 UTC (permalink / raw)
  To: kernel-janitors, linux-rdma, Daisuke Matsuda, Doug Ledford,
	Jason Gunthorpe, Leon Romanovsky, Matan Barak, Max Gurtovoy,
	Sagi Grimberg, Yishai Hadas
  Cc: cocci, LKML

Date: Thu, 6 Apr 2023 15:48:06 +0200

The label “err_put” was used to jump to another pointer check despite of
the detail in the implementation of the function “create_qp”
that it was determined already that the corresponding variable contained
an error pointer because of a failed call of the function “uobj_get_read”.

1. Thus use more appropriate labels instead.

2. Reorder jump targets at the end.

3. Split two condition checks.


This issue was detected by using the Coccinelle software.

Fixes: fd3c7904db6e05043398aee5c1448682acfb025b ("IB/core: Change idr objects to use the new schema")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/core/uverbs_cmd.c | 52 +++++++++++++++++++---------
 1 file changed, 36 insertions(+), 16 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 4796f6a8828c..465f55de4684 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1323,7 +1323,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 					    cmd->rwq_ind_tbl_handle, attrs);
 		if (!ind_tbl) {
 			ret = -EINVAL;
-			goto err_put;
+			goto abort_alloc;
 		}

 		attr.rwq_ind_tbl = ind_tbl;
@@ -1331,7 +1331,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,

 	if (ind_tbl && (cmd->max_recv_wr || cmd->max_recv_sge || cmd->is_srq)) {
 		ret = -EINVAL;
-		goto err_put;
+		goto put_obj_read_ind_tbl;
 	}

 	if (ind_tbl && !cmd->max_send_wr)
@@ -1343,13 +1343,13 @@ static int create_qp(struct uverbs_attr_bundle *attrs,

 		if (IS_ERR(xrcd_uobj)) {
 			ret = -EINVAL;
-			goto err_put;
+			goto recheck_ind_tbl;
 		}

 		xrcd = (struct ib_xrcd *)xrcd_uobj->object;
 		if (!xrcd) {
 			ret = -EINVAL;
-			goto err_put;
+			goto put_read_xrcd_uobj;
 		}
 		device = xrcd->device;
 	} else {
@@ -1360,9 +1360,14 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 			if (cmd->is_srq) {
 				srq = uobj_get_obj_read(srq, UVERBS_OBJECT_SRQ,
 							cmd->srq_handle, attrs);
-				if (!srq || srq->srq_type == IB_SRQT_XRC) {
+				if (!srq) {
+					ret = -EINVAL;
+					goto recheck_xrcd_uobj;
+				}
+
+				if (srq->srq_type == IB_SRQT_XRC) {
 					ret = -EINVAL;
-					goto err_put;
+					goto put_uobject_srq;
 				}
 			}

@@ -1373,7 +1378,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 						cmd->recv_cq_handle, attrs);
 					if (!rcq) {
 						ret = -EINVAL;
-						goto err_put;
+						goto recheck_srq;
 					}
 				}
 			}
@@ -1386,9 +1391,14 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 			rcq = rcq ?: scq;
 		pd = uobj_get_obj_read(pd, UVERBS_OBJECT_PD, cmd->pd_handle,
 				       attrs);
-		if (!pd || (!scq && has_sq)) {
+		if (!pd) {
 			ret = -EINVAL;
-			goto err_put;
+			goto recheck_scq;
+		}
+
+		if (!scq && has_sq) {
+			ret = -EINVAL;
+			goto put_obj_read_pd;
 		}

 		device = pd->device;
@@ -1422,13 +1432,13 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 				IB_QP_CREATE_SOURCE_QPN |
 				IB_QP_CREATE_PCI_WRITE_END_PADDING)) {
 		ret = -EINVAL;
-		goto err_put;
+		goto recheck_pd;
 	}

 	if (attr.create_flags & IB_QP_CREATE_SOURCE_QPN) {
 		if (!capable(CAP_NET_RAW)) {
 			ret = -EPERM;
-			goto err_put;
+			goto recheck_pd;
 		}

 		attr.source_qpn = cmd->source_qpn;
@@ -1438,7 +1448,7 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 			       KBUILD_MODNAME);
 	if (IS_ERR(qp)) {
 		ret = PTR_ERR(qp);
-		goto err_put;
+		goto recheck_pd;
 	}
 	ib_qp_usecnt_inc(qp);

@@ -1476,26 +1486,36 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 	resp.base.max_recv_wr     = attr.cap.max_recv_wr;
 	resp.base.max_send_wr     = attr.cap.max_send_wr;
 	resp.base.max_inline_data = attr.cap.max_inline_data;
+
 	resp.response_length = uverbs_response_length(attrs, sizeof(resp));
 	return uverbs_response(attrs, &resp, sizeof(resp));

-err_put:
-	if (!IS_ERR(xrcd_uobj))
-		uobj_put_read(xrcd_uobj);
+recheck_pd:
 	if (pd)
+put_obj_read_pd:
 		uobj_put_obj_read(pd);
+recheck_scq:
 	if (scq)
 		rdma_lookup_put_uobject(&scq->uobject->uevent.uobject,
 					UVERBS_LOOKUP_READ);
+recheck_rcq:
 	if (rcq && rcq != scq)
 		rdma_lookup_put_uobject(&rcq->uobject->uevent.uobject,
 					UVERBS_LOOKUP_READ);
+recheck_srq:
 	if (srq)
+put_uobject_srq:
 		rdma_lookup_put_uobject(&srq->uobject->uevent.uobject,
 					UVERBS_LOOKUP_READ);
+recheck_xrcd_uobj:
+	if (!IS_ERR(xrcd_uobj))
+put_read_xrcd_uobj:
+		uobj_put_read(xrcd_uobj);
+recheck_ind_tbl:
 	if (ind_tbl)
+put_obj_read_ind_tbl:
 		uobj_put_obj_read(ind_tbl);
-
+abort_alloc:
 	uobj_alloc_abort(&obj->uevent.uobject, attrs);
 	return ret;
 }
--
2.40.0


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

* [cocci] [PATCH 2/2] IB/uverbs: Delete a duplicate check in create_qp()
  2023-04-06 16:10   ` [cocci] [PATCH 0/2] IB/uverbs: Adjustments for create_qp() Markus Elfring
  2023-04-06 16:14     ` [cocci] [PATCH 1/2] IB/uverbs: Improve exception handling in create_qp() Markus Elfring
@ 2023-04-06 16:18     ` Markus Elfring
  1 sibling, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-06 16:18 UTC (permalink / raw)
  To: kernel-janitors, linux-rdma, Daisuke Matsuda, Doug Ledford,
	Jason Gunthorpe, Leon Romanovsky, Matan Barak, Max Gurtovoy,
	Sagi Grimberg, Yishai Hadas
  Cc: cocci, LKML

Date: Thu, 6 Apr 2023 17:30:29 +0200

The check for the variable “ind_tbl” was repeated directly after
an other if statement.

Thus move the common code into the previous if branch.

This issue was detected by using the Coccinelle software.

Fixes: c70285f880e88cb4f73effb722065a182ba5936f ("IB/uverbs: Extend create QP to get RWQ indirection table")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/core/uverbs_cmd.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 465f55de4684..bd741c507bad 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -1329,13 +1329,15 @@ static int create_qp(struct uverbs_attr_bundle *attrs,
 		attr.rwq_ind_tbl = ind_tbl;
 	}

-	if (ind_tbl && (cmd->max_recv_wr || cmd->max_recv_sge || cmd->is_srq)) {
-		ret = -EINVAL;
-		goto put_obj_read_ind_tbl;
-	}
+	if (ind_tbl) {
+		if (cmd->max_recv_wr || cmd->max_recv_sge || cmd->is_srq) {
+			ret = -EINVAL;
+			goto put_obj_read_ind_tbl;
+		}

-	if (ind_tbl && !cmd->max_send_wr)
-		has_sq = false;
+		if (!cmd->max_send_wr)
+			has_sq = false;
+	}

 	if (cmd->qp_type == IB_QPT_XRC_TGT) {
 		xrcd_uobj = uobj_get_read(UVERBS_OBJECT_XRCD, cmd->pd_handle,
--
2.40.0


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

* [cocci] [PATCH] remoteproc: imx_dsp_rproc: Improve exception handling in imx_dsp_rproc_mbox_alloc()
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
                     ` (2 preceding siblings ...)
  2023-04-06 16:10   ` [cocci] [PATCH 0/2] IB/uverbs: Adjustments for create_qp() Markus Elfring
@ 2023-04-06 20:12   ` Markus Elfring
  2023-04-07  6:22   ` [cocci] [PATCH] spi: atmel: Improve exception handling in atmel_spi_configure_dma() Markus Elfring
  2023-04-07  7:50   ` [cocci] [PATCH] ipvs: Fix exception handling in two functions Markus Elfring
  5 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-06 20:12 UTC (permalink / raw)
  To: kernel-janitors, linux-remoteproc, linux-arm-kernel, linux-imx,
	kernel, Bjorn Andersson, Fabio Estevam, Mathieu Poirier,
	Sascha Hauer, Shawn Guo
  Cc: cocci, LKML

Date: Thu, 6 Apr 2023 22:00:24 +0200

The label “err_out” was used to jump to another pointer check
despite of the detail in the implementation of the function
“imx_dsp_rproc_mbox_alloc” that it was determined already
that the corresponding variable contained an error pointer
because of a failed call of the function “mbox_request_channel_byname”.

Thus perform the following adjustments:

1. Return directly after a call of the function
   “mbox_request_channel_byname” failed for the input parameter “tx”.

2. Use more appropriate labels instead.

3. Reorder jump targets at the end.

4. Omit a function call and three extra checks.


This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/remoteproc/imx_dsp_rproc.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
index 21759d9e5b7b..a8ad15ef1da0 100644
--- a/drivers/remoteproc/imx_dsp_rproc.c
+++ b/drivers/remoteproc/imx_dsp_rproc.c
@@ -530,7 +530,7 @@ static int imx_dsp_rproc_mbox_alloc(struct imx_dsp_rproc *priv)
 		ret = PTR_ERR(priv->tx_ch);
 		dev_dbg(cl->dev, "failed to request tx mailbox channel: %d\n",
 			ret);
-		goto err_out;
+		return ret;
 	}

 	/* Channel for receiving message */
@@ -539,7 +539,7 @@ static int imx_dsp_rproc_mbox_alloc(struct imx_dsp_rproc *priv)
 		ret = PTR_ERR(priv->rx_ch);
 		dev_dbg(cl->dev, "failed to request rx mailbox channel: %d\n",
 			ret);
-		goto err_out;
+		goto free_channel_tx;
 	}

 	cl = &priv->cl_rxdb;
@@ -555,19 +555,15 @@ static int imx_dsp_rproc_mbox_alloc(struct imx_dsp_rproc *priv)
 		ret = PTR_ERR(priv->rxdb_ch);
 		dev_dbg(cl->dev, "failed to request mbox chan rxdb, ret %d\n",
 			ret);
-		goto err_out;
+		goto free_channel_rx;
 	}

 	return 0;

-err_out:
-	if (!IS_ERR(priv->tx_ch))
-		mbox_free_channel(priv->tx_ch);
-	if (!IS_ERR(priv->rx_ch))
-		mbox_free_channel(priv->rx_ch);
-	if (!IS_ERR(priv->rxdb_ch))
-		mbox_free_channel(priv->rxdb_ch);
-
+free_channel_rx:
+	mbox_free_channel(priv->rx_ch);
+free_channel_tx:
+	mbox_free_channel(priv->tx_ch);
 	return ret;
 }

--
2.40.0


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

* [cocci] [PATCH] spi: atmel: Improve exception handling in atmel_spi_configure_dma()
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
                     ` (3 preceding siblings ...)
  2023-04-06 20:12   ` [cocci] [PATCH] remoteproc: imx_dsp_rproc: Improve exception handling in imx_dsp_rproc_mbox_alloc() Markus Elfring
@ 2023-04-07  6:22   ` Markus Elfring
       [not found]     ` <e3e704f5-3719-637a-8bc5-3a24e2e7cb92@microchip.com>
  2023-04-07  7:50   ` [cocci] [PATCH] ipvs: Fix exception handling in two functions Markus Elfring
  5 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-04-07  6:22 UTC (permalink / raw)
  To: kernel-janitors, linux-spi, linux-arm-kernel, Alexandre Belloni,
	Claudiu Beznea, Mark Brown, Nicolas Ferre, Tudor Ambarus,
	Yang Yingliang
  Cc: cocci, LKML

Date: Fri, 7 Apr 2023 08:08:59 +0200

The label “error” was used to jump to another pointer check despite of
the detail in the implementation of the function “atmel_spi_configure_dma”
that it was determined already that the corresponding variable
contained an error pointer because of a failed call of
the function “dma_request_chan”.

* Thus use more appropriate labels instead.

* Delete two redundant checks.


This issue was detected by using the Coccinelle software.

Fixes: 398b6b310ec85eef9d98df5963d5ded18aa92ad8 ("spi: atmel: switch to use modern name")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/spi/spi-atmel.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 7f06305e16cb..ed8dc93c73e5 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -511,12 +511,12 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
 		 * requested tx channel.
 		 */
 		dev_dbg(dev, "No RX DMA channel, DMA is disabled\n");
-		goto error;
+		goto release_channel_tx;
 	}

 	err = atmel_spi_dma_slave_config(as, 8);
 	if (err)
-		goto error;
+		goto release_channel_rx;

 	dev_info(&as->pdev->dev,
 			"Using %s (tx) and %s (rx) for DMA transfers\n",
@@ -524,11 +524,11 @@ static int atmel_spi_configure_dma(struct spi_controller *host,
 			dma_chan_name(host->dma_rx));

 	return 0;
-error:
-	if (!IS_ERR(host->dma_rx))
-		dma_release_channel(host->dma_rx);
-	if (!IS_ERR(host->dma_tx))
-		dma_release_channel(host->dma_tx);
+
+release_channel_rx:
+	dma_release_channel(host->dma_rx);
+release_channel_tx:
+	dma_release_channel(host->dma_tx);
 error_clear:
 	host->dma_tx = host->dma_rx = NULL;
 	return err;
--
2.40.0


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

* [cocci] [PATCH] ipvs: Fix exception handling in two functions
  2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
                     ` (4 preceding siblings ...)
  2023-04-07  6:22   ` [cocci] [PATCH] spi: atmel: Improve exception handling in atmel_spi_configure_dma() Markus Elfring
@ 2023-04-07  7:50   ` Markus Elfring
  5 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-07  7:50 UTC (permalink / raw)
  To: kernel-janitors, netfilter-devel, netdev, lvs-devel, coreteam,
	Alex Gartrell, David S. Miller, Eric Dumazet, Florian Westphal,
	Jakub Kicinski, Jozsef Kadlecsik, Julian Anastasov,
	Pablo Neira Ayuso, Paolo Abeni, Simon Horman
  Cc: cocci, LKML

Date: Fri, 7 Apr 2023 09:33:43 +0200

The label “tx_error” was used to jump to another pointer check
despite of the detail in these function implementations
that it was determined already that the corresponding variable
contained an error pointer because of a failed call of
the function “ip_vs_prepare_tunneled_skb”.

* Thus use an additional label.

* Delete two redundant checks.


This issue was detected by using the Coccinelle software.

Fixes: ea1d5d7755a3e556de78cc757d1895d5c7180548 ("ipvs: properly declare tunnel encapsulation")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/netfilter/ipvs/ip_vs_xmit.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 99c349c0d968..68343ee380e1 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -1199,7 +1199,7 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 					 &next_protocol, NULL, &dsfield,
 					 &ttl, dfp);
 	if (IS_ERR(skb))
-		goto tx_error;
+		goto leave_function;

 	gso_type = __tun_gso_type_mask(AF_INET, cp->af);
 	if (tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
@@ -1266,16 +1266,14 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 		ip_local_out(net, skb->sk, skb);
 	else if (ret == NF_DROP)
 		kfree_skb(skb);
-
+leave_function:
 	LeaveFunction(10);

 	return NF_STOLEN;

   tx_error:
-	if (!IS_ERR(skb))
-		kfree_skb(skb);
-	LeaveFunction(10);
-	return NF_STOLEN;
+	kfree_skb(skb);
+	goto leave_function;
 }

 #ifdef CONFIG_IP_VS_IPV6
@@ -1347,7 +1345,7 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 					 &next_protocol, &payload_len,
 					 &dsfield, &ttl, NULL);
 	if (IS_ERR(skb))
-		goto tx_error;
+		goto leave_function;

 	gso_type = __tun_gso_type_mask(AF_INET6, cp->af);
 	if (tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
@@ -1413,16 +1411,14 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 		ip6_local_out(net, skb->sk, skb);
 	else if (ret == NF_DROP)
 		kfree_skb(skb);
-
+leave_function:
 	LeaveFunction(10);

 	return NF_STOLEN;

 tx_error:
-	if (!IS_ERR(skb))
-		kfree_skb(skb);
-	LeaveFunction(10);
-	return NF_STOLEN;
+	kfree_skb(skb);
+	goto leave_function;
 }
 #endif

--
2.40.0


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

* Re: [cocci] [PATCH] spi: atmel: Improve exception handling in atmel_spi_configure_dma()
       [not found]     ` <e3e704f5-3719-637a-8bc5-3a24e2e7cb92@microchip.com>
@ 2023-04-07 15:07       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-07 15:07 UTC (permalink / raw)
  To: Nicolas Ferre, kernel-janitors, linux-spi, linux-arm-kernel,
	Alexandre Belloni, Claudiu Beznea, Mark Brown, Tudor Ambarus,
	Yang Yingliang
  Cc: cocci, LKML

>> The label “error” was used to jump to another pointer check despite of
>> the detail in the implementation of the function “atmel_spi_configure_dma”
>> that it was determined already that the corresponding variable
>> contained an error pointer because of a failed call of
>> the function “dma_request_chan”.
>>
>> * Thus use more appropriate labels instead.
>>
>> * Delete two redundant checks.
>>
>>
>> This issue was detected by using the Coccinelle software.
>>
>> Fixes: 398b6b310ec85eef9d98df5963d5ded18aa92ad8 ("spi: atmel: switch to use modern name")
> It's becoming a pattern, but still:
> NACK.

What does hinder you to work with more jump labels for improved exception handling?

Regards,
Markus

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

* [cocci] [PATCH v2] cpufreq: sparc: Fix exception handling in two functions
       [not found]         ` <20230403230432.xeubpa3cc2gt4mw3@vireshk-i7>
@ 2023-04-07 17:48           ` Markus Elfring
       [not found]             ` <20230409235511.7xxqdxsqtflrhifk@vireshk-i7>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-04-07 17:48 UTC (permalink / raw)
  To: kernel-janitors, linux-pm, Rafael J. Wysocki, Viresh Kumar; +Cc: cocci, LKML

Date: Fri, 7 Apr 2023 19:30:12 +0200

The label “err_out” was used to jump to another pointer check despite of
the detail in the implementation of the functions “us2e_freq_init”
and “us3_freq_init” that it was determined already that the corresponding
variable contained a null pointer because of a failed memory allocation.

Thus perform the following adjustments:

1. Return directly after the first call of the function “kzalloc” failed.

2. Use an additional label.

3. Reorder jump targets at the end.

4. Move the assignment of “-ENOMEM” into two if branches.

5. Omit the assignment of NULL for four variables.

6. Delete two extra pointer checks which became unnecessary.


This issue was detected by using the Coccinelle software.

Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
V2:
Further changes were requested by Viresh Kumar.

 drivers/cpufreq/sparc-us2e-cpufreq.c | 22 +++++++++-------------
 drivers/cpufreq/sparc-us3-cpufreq.c  | 22 +++++++++-------------
 2 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/drivers/cpufreq/sparc-us2e-cpufreq.c b/drivers/cpufreq/sparc-us2e-cpufreq.c
index 92acbb25abb3..f1ab085894a4 100644
--- a/drivers/cpufreq/sparc-us2e-cpufreq.c
+++ b/drivers/cpufreq/sparc-us2e-cpufreq.c
@@ -319,17 +319,17 @@ static int __init us2e_freq_init(void)
 	impl  = ((ver >> 32) & 0xffff);

 	if (manuf == 0x17 && impl == 0x13) {
-		struct cpufreq_driver *driver;
+		struct cpufreq_driver *driver = kzalloc(sizeof(*driver), GFP_KERNEL);

-		ret = -ENOMEM;
-		driver = kzalloc(sizeof(*driver), GFP_KERNEL);
 		if (!driver)
-			goto err_out;
+			return -ENOMEM;

 		us2e_freq_table = kzalloc((NR_CPUS * sizeof(*us2e_freq_table)),
 			GFP_KERNEL);
-		if (!us2e_freq_table)
-			goto err_out;
+		if (!us2e_freq_table) {
+			ret = -ENOMEM;
+			goto free_driver;
+		}

 		driver->init = us2e_freq_cpu_init;
 		driver->verify = cpufreq_generic_frequency_table_verify;
@@ -337,21 +337,17 @@ static int __init us2e_freq_init(void)
 		driver->get = us2e_freq_get;
 		driver->exit = us2e_freq_cpu_exit;
 		strcpy(driver->name, "UltraSPARC-IIe");
-
-		cpufreq_us2e_driver = driver;
 		ret = cpufreq_register_driver(driver);
 		if (ret)
 			goto err_out;

+		cpufreq_us2e_driver = driver;
 		return 0;

 err_out:
-		if (driver) {
-			kfree(driver);
-			cpufreq_us2e_driver = NULL;
-		}
 		kfree(us2e_freq_table);
-		us2e_freq_table = NULL;
+free_driver:
+		kfree(driver);
 		return ret;
 	}

diff --git a/drivers/cpufreq/sparc-us3-cpufreq.c b/drivers/cpufreq/sparc-us3-cpufreq.c
index e41b35b16afd..d603366deb7b 100644
--- a/drivers/cpufreq/sparc-us3-cpufreq.c
+++ b/drivers/cpufreq/sparc-us3-cpufreq.c
@@ -167,17 +167,17 @@ static int __init us3_freq_init(void)
 	     impl == CHEETAH_PLUS_IMPL ||
 	     impl == JAGUAR_IMPL ||
 	     impl == PANTHER_IMPL)) {
-		struct cpufreq_driver *driver;
+		struct cpufreq_driver *driver = kzalloc(sizeof(*driver), GFP_KERNEL);

-		ret = -ENOMEM;
-		driver = kzalloc(sizeof(*driver), GFP_KERNEL);
 		if (!driver)
-			goto err_out;
+			return -ENOMEM;

 		us3_freq_table = kzalloc((NR_CPUS * sizeof(*us3_freq_table)),
 			GFP_KERNEL);
-		if (!us3_freq_table)
-			goto err_out;
+		if (!us3_freq_table) {
+			ret = -ENOMEM;
+			goto free_driver;
+		}

 		driver->init = us3_freq_cpu_init;
 		driver->verify = cpufreq_generic_frequency_table_verify;
@@ -185,21 +185,17 @@ static int __init us3_freq_init(void)
 		driver->get = us3_freq_get;
 		driver->exit = us3_freq_cpu_exit;
 		strcpy(driver->name, "UltraSPARC-III");
-
-		cpufreq_us3_driver = driver;
 		ret = cpufreq_register_driver(driver);
 		if (ret)
 			goto err_out;

+		cpufreq_us3_driver = driver;
 		return 0;

 err_out:
-		if (driver) {
-			kfree(driver);
-			cpufreq_us3_driver = NULL;
-		}
 		kfree(us3_freq_table);
-		us3_freq_table = NULL;
+free_driver:
+		kfree(driver);
 		return ret;
 	}

--
2.40.0


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

* Re: [cocci] [v2] cpufreq: sparc: Fix exception handling in two functions
       [not found]             ` <20230409235511.7xxqdxsqtflrhifk@vireshk-i7>
@ 2023-04-10 13:08               ` Markus Elfring
       [not found]                 ` <20230411033048.zwsijlyiksjcmgcc@vireshk-i7>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-04-10 13:08 UTC (permalink / raw)
  To: Viresh Kumar, linux-pm, kernel-janitors; +Cc: Rafael J. Wysocki, cocci, LKML

>> @@ -337,21 +337,17 @@ static int __init us2e_freq_init(void)
>>  		driver->get = us2e_freq_get;
>>  		driver->exit = us2e_freq_cpu_exit;
>>  		strcpy(driver->name, "UltraSPARC-IIe");
>> -
>> -		cpufreq_us2e_driver = driver;
>
> This changes the behavior of the code here as "cpufreq_us2e_driver"
> is used in us2e_freq_cpu_exit(). If some failure occurs after a
> policy is initialized, and driver doesn't register successfully, then
> we won't set the frequency to the lowest index of the table anymore.

The setting of the variables “cpufreq_us…_driver” influences the need
to reset them to null pointers for the desired exception handling,
doesn't it?

Regards,
Markus

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

* Re: [cocci] [v2] cpufreq: sparc: Fix exception handling in two functions
       [not found]                 ` <20230411033048.zwsijlyiksjcmgcc@vireshk-i7>
@ 2023-04-11  6:15                   ` Markus Elfring
       [not found]                     ` <20230411064051.qyioheeoectj2lv3@vireshk-i7>
  0 siblings, 1 reply; 99+ messages in thread
From: Markus Elfring @ 2023-04-11  6:15 UTC (permalink / raw)
  To: Viresh Kumar, Rafael J. Wysocki, linux-pm, kernel-janitors; +Cc: cocci, LKML

>> The setting of the variables “cpufreq_us…_driver” influences the need
>> to reset them to null pointers for the desired exception handling,
>> doesn't it?
>
> This is what all should be done for these drivers I guess. There is no
> points doing the dance of {de}allocating resources unnecessarily.

Are you going to integrate your source code adjustment according to
reduced dynamic memory allocation?

Regards,
Markus

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

* Re: [cocci] [v2] cpufreq: sparc: Fix exception handling in two functions
       [not found]                     ` <20230411064051.qyioheeoectj2lv3@vireshk-i7>
@ 2023-04-11 14:20                       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-04-11 14:20 UTC (permalink / raw)
  To: Viresh Kumar, linux-pm; +Cc: Rafael J. Wysocki, cocci, LKML, kernel-janitors

>>> This is what all should be done for these drivers I guess. There is no
>>> points doing the dance of {de}allocating resources unnecessarily.
>>
>> Are you going to integrate your source code adjustment according to
>> reduced dynamic memory allocation?
>
> …, else I will do it.

This change suggestion seems to be based more on your ideas.
Thus I would find it nice if you can generate a patch accordingly.

Regards,
Markus

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

* Re: [cocci] firmware: ti_sci: Fix exception handling in ti_sci_probe()
       [not found]     ` <20230516152043.bq2hnessstrhbc6s@distort>
@ 2023-05-16 15:56       ` Markus Elfring
  0 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2023-05-16 15:56 UTC (permalink / raw)
  To: Nishanth Menon, kernel-janitors, linux-arm-kernel
  Cc: Santosh Shilimkar, Tero Kristo, cocci, LKML

>> This issue was detected by using the Coccinelle software.
>
> Curious: what rule of coccicheck caught this?

None. (So far)

See also:
Reconsidering repeated checks for error pointers (with SmPL)
2023-04-04
https://lore.kernel.org/cocci/8f785de5-ebe2-edd9-2155-f440acacc643@web.de/
https://sympa.inria.fr/sympa/arc/cocci/2023-04/msg00008.html

Regards,
Markus

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

* Re: [cocci] [PATCH resent v2 0/2] powerpc/pseries: Fixes for exception handling in pSeries_reconfig_add_node()
  2023-03-25 13:40                 ` [cocci] [PATCH resent " Markus Elfring
  2023-03-25 13:42                   ` [cocci] [PATCH resent v2 1/2] powerpc/pseries: Do not pass an error pointer to of_node_put() " Markus Elfring
  2023-03-25 13:44                   ` [cocci] [PATCH resent v2 2/2] powerpc/pseries: Fix exception handling " Markus Elfring
@ 2024-01-05 17:19                   ` Markus Elfring
  2 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2024-01-05 17:19 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Christophe Leroy,
	Michael Ellerman, Nathan Lynch, Nicholas Piggin, Paul Moore
  Cc: cocci, LKML

> Date: Tue, 21 Mar 2023 11:26:32 +0100
>
> A few update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (2):
>   Do not pass an error pointer to of_node_put()
>   Fix exception handling
>
>  arch/powerpc/platforms/pseries/reconfig.c | 26 ++++++++++++-----------
>  1 file changed, 14 insertions(+), 12 deletions(-)

Is this patch series still in review queues?

Regards,
Markus

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

* Re: [cocci] [PATCH v2 0/4] powerpc/4xx: Adjustments for four function implementations
  2023-03-25 15:30   ` [cocci] [PATCH v2 " Markus Elfring
                       ` (3 preceding siblings ...)
  2023-03-25 15:42     ` [cocci] [PATCH v2 4/4] powerpc/4xx: Delete unnecessary variable initialisations in four functions Markus Elfring
@ 2024-01-05 17:42     ` Markus Elfring
  4 siblings, 0 replies; 99+ messages in thread
From: Markus Elfring @ 2024-01-05 17:42 UTC (permalink / raw)
  To: kernel-janitors, linuxppc-dev, Benjamin Herrenschmidt,
	Christophe Leroy, Josh Boyer, Michael Ellerman, Nicholas Piggin,
	Stefan Roese
  Cc: cocci, LKML

> Date: Sat, 25 Mar 2023 16:10:23 +0100
>
> Some update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (4):
>   Fix exception handling in ppc4xx_pciex_port_setup_hose()
>   Fix exception handling in ppc4xx_probe_pcix_bridge()
>   Fix exception handling in ppc4xx_probe_pci_bridge()
>   Delete unnecessary variable initialisations in four functions
>
>  arch/powerpc/platforms/4xx/pci.c | 69 ++++++++++++++------------------
>  1 file changed, 31 insertions(+), 38 deletions(-)

Is this patch series still in review queues?

Regards,
Markus

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

end of thread, other threads:[~2024-01-05 17:43 UTC | newest]

Thread overview: 99+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-15 17:36 [cocci] Reconsidering repeated pointer checks with SmPL Markus Elfring
2023-03-16 20:07 ` [cocci] [PATCH 0/4] powerpc/4xx: Adjustments for four function implementations Markus Elfring
2023-03-25 15:30   ` [cocci] [PATCH v2 " Markus Elfring
2023-03-25 15:36     ` [cocci] [PATCH v2 1/4] powerpc/4xx: Fix exception handling in ppc4xx_pciex_port_setup_hose() Markus Elfring
2023-03-25 15:38     ` [cocci] [PATCH v2 2/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pcix_bridge() Markus Elfring
2023-03-25 15:40     ` [cocci] [PATCH v2 3/4] powerpc/4xx: Fix exception handling in ppc4xx_probe_pci_bridge() Markus Elfring
2023-03-25 15:42     ` [cocci] [PATCH v2 4/4] powerpc/4xx: Delete unnecessary variable initialisations in four functions Markus Elfring
2024-01-05 17:42     ` [cocci] [PATCH v2 0/4] powerpc/4xx: Adjustments for four function implementations Markus Elfring
     [not found] ` <0981dc33-95d0-4a1b-51d9-168907da99e6@web.de>
2023-03-17 13:11   ` [cocci] [PATCH] powerpc/pseries: Fix exception handling in pSeries_reconfig_add_node() Nathan Lynch
2023-03-17 14:20     ` [cocci] " Markus Elfring
2023-03-17 15:41       ` Nathan Lynch
2023-03-18  7:30         ` Markus Elfring
2023-03-20 15:38           ` Nathan Lynch
2023-03-21  6:54             ` Markus Elfring
2023-03-21 10:30               ` [cocci] [PATCH v2 0/2] powerpc/pseries: Fixes for " Markus Elfring
2023-03-25 13:40                 ` [cocci] [PATCH resent " Markus Elfring
2023-03-25 13:42                   ` [cocci] [PATCH resent v2 1/2] powerpc/pseries: Do not pass an error pointer to of_node_put() " Markus Elfring
2023-03-25 13:44                   ` [cocci] [PATCH resent v2 2/2] powerpc/pseries: Fix exception handling " Markus Elfring
2024-01-05 17:19                   ` [cocci] [PATCH resent v2 0/2] powerpc/pseries: Fixes for " Markus Elfring
     [not found] ` <afe30fc6-04c9-528c-f84a-67902b5a6ed8@web.de>
2023-03-19 11:40   ` [cocci] [PATCH] RDMA/siw: Fix exception handling in siw_accept_newconn() Leon Romanovsky
     [not found]     ` <1c06e86d-1468-c11a-8344-9563ad6047b5@web.de>
2023-03-19 14:11       ` Leon Romanovsky
     [not found]         ` <a03c1d04-a41e-7722-c36a-bd6f61094702@web.de>
     [not found]           ` <20230319173716.GF36557@unreal>
2023-03-20  8:19             ` [cocci] " Markus Elfring
     [not found] ` <f0f96f74-21d1-f5bf-1086-1c3ce0ea18f5@web.de>
2023-03-19 11:41   ` [cocci] [PATCH] RDMA/erdma: Fix exception handling in erdma_accept_newconn() Leon Romanovsky
2023-03-19 13:36   ` Cheng Xu
2023-03-19 14:00     ` Markus Elfring
2023-03-19 20:00 ` [cocci] [PATCH 0/2] irqchip/gic-v4: Adjustments for two function implementations Markus Elfring
2023-03-25 10:25   ` [cocci] [PATCH resent " Markus Elfring
2023-03-25 10:27     ` [cocci] [PATCH resent 1/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_irqs() Markus Elfring
2023-03-25 10:30     ` [cocci] [PATCH resent 2/2] irqchip/gic-v4: Fix exception handling in its_alloc_vcpu_sgis() Markus Elfring
     [not found] ` <521b63e1-9470-58ef-599e-50a1846e5380@web.de>
     [not found]   ` <ZBffPEIWcmYcaXR3@google.com>
2023-03-20  8:55     ` [cocci] [PATCH] Input: iforce - Fix exception handling in iforce_usb_probe() Markus Elfring
     [not found]     ` <6b5de584-b31f-9045-a438-b42da350326b@I-love.SAKURA.ne.jp>
     [not found]       ` <ZBf3oSgJP8bLmhG0@google.com>
2023-03-25 10:12         ` [cocci] [PATCH resent] " Markus Elfring
2023-03-20 16:47 ` [cocci] [PATCH 0/2] md/raid: Adjustments for two function implementations Markus Elfring
2023-03-25  9:20   ` [cocci] [PATCH resent " Markus Elfring
2023-03-25  9:22     ` [cocci] [PATCH resent 1/2] md/raid1: Fix exception handling in setup_conf() Markus Elfring
2023-03-25  9:24     ` [cocci] [PATCH resent 2/2] md/raid10: " Markus Elfring
     [not found]     ` <CAPhsuW7JZDps_fTHyCabjfG4YjzDVEW_41u6d+9mdc2CAJv_Kw@mail.gmail.com>
2023-03-29  5:32       ` [cocci] [0/2] md/raid: Adjustments for two function implementations Markus Elfring
2023-03-21 16:07 ` [cocci] [PATCH 0/3] scsi: message: fusion: Adjustments for four " Markus Elfring
2023-03-25  9:00   ` [cocci] [PATCH resent " Markus Elfring
2023-03-25  9:03     ` [cocci] [PATCH resent 1/3] scsi: message: fusion: Return directly after input data validation failed in four functions Markus Elfring
2023-03-25  9:07     ` [cocci] [PATCH resent 2/3] scsi: message: fusion: Delete a redundant pointer check " Markus Elfring
2023-03-25  9:10     ` [cocci] [PATCH resent 3/3] scsi: message: fusion: Delete an unnecessary variable initialisation " Markus Elfring
     [not found] ` <e3aaeecf-8e74-2e74-c58a-d80e153e98f9@web.de>
2023-03-22  9:36   ` [cocci] [PATCH] media: hantro: HEVC: Fix exception handling in tile_buffer_reallocate() Benjamin Gaignard
2023-03-22 14:45     ` Markus Elfring
2023-03-22 19:20 ` [cocci] [PATCH] btrfs: Fix exception handling in relocating_repair_kthread() Markus Elfring
2023-03-25  8:25   ` [cocci] [PATCH resent] " Markus Elfring
2023-03-22 21:20 ` [cocci] [PATCH] ufs: Fix exception handling in ufs_fill_super() Markus Elfring
2023-03-25  8:32   ` [cocci] [PATCH resent] " Markus Elfring
2023-03-23 17:30 ` [cocci] [PATCH] mm/mempolicy: Fix exception handling in shared_policy_replace() Markus Elfring
2023-03-24 17:30   ` Vlastimil Babka
2023-03-24 18:03     ` Markus Elfring
2023-03-23 21:12 ` [cocci] [PATCH] perf cputopo: Improve exception handling in build_cpu_topology() Markus Elfring
2023-03-25  8:50   ` [cocci] [PATCH resent] " Markus Elfring
2023-03-24 11:43 ` [cocci] [PATCH] perf pmu: Improve exception handling in pmu_lookup() Markus Elfring
2023-03-24 14:12 ` [cocci] [PATCH] selftests/bpf: Improve exception handling in rbtree_add_and_remove() Markus Elfring
     [not found] ` <e6656c83-ee7a-a253-2028-109138779c94@web.de>
2023-03-24 15:42   ` [cocci] [PATCH resent] drm/amd/display: Fix exception handling in dm_validate_stream_and_context() Markus Elfring
     [not found]     ` <32674bac-92c2-8fc7-0977-6d2d81b3257f@amd.com>
2023-03-24 18:19       ` Markus Elfring
     [not found] ` <7214c986-4d0e-ad78-6312-c84515dc9abf@web.de>
2023-03-24 17:02   ` [cocci] [PATCH resent] ext4: Fix exception handling in parse_apply_sb_mount_options() Markus Elfring
     [not found] ` <e33f264a-7ee9-4ebc-d58e-bbb7fd567198@web.de>
2023-03-25  9:31   ` [cocci] [PATCH resent] bcache: Fix exception handling in mca_alloc() Markus Elfring
     [not found]     ` <157b8db9-82f7-85e7-3bbd-7ef3a1797892@suse.de>
2023-03-25 12:21       ` [cocci] [PATCH v2] " Markus Elfring
     [not found]         ` <BE6CEE57-E9AF-4F17-B281-1E00C5DC2A9C@suse.de>
2023-03-25 17:10           ` [cocci] " Markus Elfring
2023-03-25 12:50       ` [cocci] [PATCH resent] " Markus Elfring
     [not found] ` <00589154-00ac-4ed5-2a37-60b3c6f6c523@web.de>
2023-03-25  9:40   ` [cocci] [PATCH resent] mei: Fix exception handling in mei_cl_irq_read_msg() Markus Elfring
     [not found] ` <3675f707-bff0-3caf-29a2-b99e5b9c6554@web.de>
2023-03-25  9:43   ` [cocci] [PATCH resent] mtd: cfi_cmdset_0001: Fix exception handling in cfi_intelext_setup() Markus Elfring
     [not found] ` <21e58abb-f215-b9b7-ffe8-236dd40c6bd2@web.de>
2023-03-25  9:50   ` [cocci] [PATCH resent] bbc_i2c: Fix exception handling in attach_one_i2c() Markus Elfring
     [not found] ` <15fa53e5-916f-dac8-87fb-9cb81021418c@web.de>
2023-03-25 10:20   ` [cocci] [PATCH resent] irqchip/partitions: Fix exception handling in partition_create_desc() Markus Elfring
     [not found] ` <eaa9f90f-c91b-dc87-51a1-d97f99fc5b4b@web.de>
2023-03-25 14:00   ` [cocci] [PATCH resent] dmaengine: bestcomm: Fix exception handling in bcom_task_alloc() Markus Elfring
     [not found] ` <b3cce5b3-2e68-180c-c293-74d4d9d4032c@web.de>
2023-03-25 14:02   ` [cocci] [PATCH resent] cpufreq: sparc: Fix exception handling in two functions Markus Elfring
     [not found]     ` <20230403033529.x6n3ihhkypwizq3b@vireshk-i7>
2023-04-03 12:50       ` [cocci] [PATCH] " Markus Elfring
     [not found]         ` <20230403230432.xeubpa3cc2gt4mw3@vireshk-i7>
2023-04-07 17:48           ` [cocci] [PATCH v2] " Markus Elfring
     [not found]             ` <20230409235511.7xxqdxsqtflrhifk@vireshk-i7>
2023-04-10 13:08               ` [cocci] [v2] " Markus Elfring
     [not found]                 ` <20230411033048.zwsijlyiksjcmgcc@vireshk-i7>
2023-04-11  6:15                   ` Markus Elfring
     [not found]                     ` <20230411064051.qyioheeoectj2lv3@vireshk-i7>
2023-04-11 14:20                       ` Markus Elfring
     [not found] ` <5ed1bc78-77a1-8eb8-43f9-6005d7de49c8@web.de>
2023-03-25 14:05   ` [cocci] [PATCH resent] clk: at91: sama7g5: Add two jump labels in sama7g5_pmc_setup() Markus Elfring
     [not found]     ` <7890284f-5809-2f46-9d5f-52e20a3ec327@microchip.com>
2023-03-28 19:24       ` Markus Elfring
     [not found] ` <8d193937-532f-959f-9b84-d911984508aa@web.de>
2023-03-25 14:07   ` [cocci] [PATCH resent] drbd: Fix exception handling in nla_put_drbd_cfg_context() Markus Elfring
2023-03-27 12:28     ` Christoph Böhmwalder
2023-03-27 17:26       ` [cocci] " Markus Elfring
     [not found] ` <3151f1ef-63c6-d016-7c6a-2572e3d93d8f@web.de>
2023-03-25 14:10   ` [cocci] [PATCH resent] blk-mq: Add two labels in blk_rq_prep_clone() Markus Elfring
     [not found] ` <ab860edf-79ca-2035-c5a3-d25be6fd9dac@web.de>
2023-03-25 14:15   ` [cocci] [PATCH resent] perf/x86/amd/uncore: Fix exception handling in amd_uncore_cpu_up_prepare() Markus Elfring
     [not found]     ` <d691d740-c172-a5cb-e4f0-5bc5687c8464@intel.com>
2023-03-27 17:36       ` [cocci] " Markus Elfring
2023-03-25 18:30 ` [cocci] [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events() Markus Elfring
     [not found]   ` <fffcd98a-bb73-41cd-8545-0f2c55dd38f9@lucifer.local>
2023-03-26  8:15     ` Markus Elfring
     [not found]       ` <20230326213900.GJ363182@maniforge>
2023-03-27  5:56         ` Markus Elfring
2023-03-26  8:20 ` [cocci] [PATCH] selinux: Improve exception handling in security_get_bools() Markus Elfring
     [not found]   ` <tencent_1C472DFEAAEC903262297C9B9C978B365909@qq.com>
2023-03-27  5:25     ` [cocci] " Markus Elfring
2023-03-27  7:00   ` [cocci] [PATCH v2] selinux: Adjust implementation of security_get_bools() Markus Elfring
     [not found]     ` <CAHC9VhR=yK72JXW3hJR+gUQtGCNpF0Bzk5RDzPZR0MunC84AUQ@mail.gmail.com>
     [not found]       ` <CAHC9VhREfdgiCji=uEeCrc4w1kPGfnWGKnJuUYKXwTApdneSjQ@mail.gmail.com>
2023-03-28  7:30         ` [cocci] " Markus Elfring
     [not found]           ` <CAHC9VhQfiNd_4uWBmKCC81UnOJb7Y=UFCDMXuqz3=UPr8QtqNw@mail.gmail.com>
2023-03-29  5:20             ` Markus Elfring
2023-04-04  8:00 ` [cocci] Reconsidering repeated checks for error pointers (with SmPL) Markus Elfring
2023-04-05 17:10   ` [cocci] [PATCH] drm/nouveau: Add a jump label in nouveau_gem_ioctl_pushbuf() Markus Elfring
2023-04-05 20:10   ` [cocci] [PATCH] firmware: ti_sci: Fix exception handling in ti_sci_probe() Markus Elfring
     [not found]     ` <20230516152043.bq2hnessstrhbc6s@distort>
2023-05-16 15:56       ` [cocci] " Markus Elfring
2023-04-06 16:10   ` [cocci] [PATCH 0/2] IB/uverbs: Adjustments for create_qp() Markus Elfring
2023-04-06 16:14     ` [cocci] [PATCH 1/2] IB/uverbs: Improve exception handling in create_qp() Markus Elfring
2023-04-06 16:18     ` [cocci] [PATCH 2/2] IB/uverbs: Delete a duplicate check " Markus Elfring
2023-04-06 20:12   ` [cocci] [PATCH] remoteproc: imx_dsp_rproc: Improve exception handling in imx_dsp_rproc_mbox_alloc() Markus Elfring
2023-04-07  6:22   ` [cocci] [PATCH] spi: atmel: Improve exception handling in atmel_spi_configure_dma() Markus Elfring
     [not found]     ` <e3e704f5-3719-637a-8bc5-3a24e2e7cb92@microchip.com>
2023-04-07 15:07       ` Markus Elfring
2023-04-07  7:50   ` [cocci] [PATCH] ipvs: Fix exception handling in two functions Markus Elfring

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