Markus Elfring <Markus.Elfring@xxxxxx> 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?
