From:   Nathan Lynch <nathanl@linux.ibm.com>
To:     Markus Elfring <Markus.Elfring@web.de>
Cc:     cocci@inria.fr, LKML <linux-kernel@vger.kernel.org>,
        Christophe Leroy <christophe.leroy@csgroup.eu>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Nicholas Piggin <npiggin@gmail.com>,
        Paul Moore <paul@paul-moore.com>,
        linuxppc-dev@lists.ozlabs.org, kernel-janitors@vger.kernel.org
Subject: Re: powerpc/pseries: Fix exception handling in
 pSeries_reconfig_add_node()
In-Reply-To: <a01643fd-1e4a-1183-2fa6-000465bc81f3@web.de>
References: <f9303bdc-b1a7-be5e-56c6-dfa8232b8b55@web.de>
 <0981dc33-95d0-4a1b-51d9-168907da99e6@web.de>
 <871qln8quw.fsf@linux.ibm.com>
 <a01643fd-1e4a-1183-2fa6-000465bc81f3@web.de>
Date:   Fri, 17 Mar 2023 10:41:31 -0500
Message-ID: <87v8iz75ck.fsf@linux.ibm.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Precedence: bulk

Markus Elfring <Markus.Elfring@web.de> writes:
>>> The label =E2=80=9Cout_err=E2=80=9D was used to jump to another pointer=
 check despite of
>>> the detail in the implementation of the function =E2=80=9CpSeries_recon=
fig_add_node=E2=80=9D
>>> 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 =E2=80=9Ckzalloc=
=E2=80=9D failed.
>>>
>>> 2. Use more appropriate labels instead.
>>>
>>> 3. Delete a redundant check.
>>>
>>> 4. Omit an explicit initialisation for the local variable =E2=80=9Cerr=
=E2=80=9D.
>>>
>>> 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 =E2=
=80=9Cout_err=E2=80=9D
> 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 i=
dentify 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. :-)