We should check the return value of kzalloc, as kzalloc may fail returning NULL.
---
v4->v5:
Separate the patch into two.
v3->v4:
Added the information about coccinelle script.
Added change log.
Added Oliver's Reviewed-by.
v2->v3:
Removed redundant assignment to 'attr' and 'name'.
v1->v2:
Just return -ENOMEM if attr is NULL.
Qiujun Huang (2):
powerpc/powernv: Remove redundant assignments to attr and name
powerpc/powernv: Add NULL check after kzalloc in opal_add_one_export
arch/powerpc/platforms/powernv/opal.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
--
2.17.1
We don't need to go to the labal of out when of_property_read_u64_array
fails, as there is nothing to do. Just return.
And we can remove the redundant assignments to attr and name.
Signed-off-by: Qiujun Huang <[email protected]>
Reviewed-by: Oliver O'Halloran <[email protected]>
---
arch/powerpc/platforms/powernv/opal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 2b3dfd0b6cdd..71af1cbc6334 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -801,14 +801,14 @@ static ssize_t export_attr_read(struct file *fp, struct kobject *kobj,
static int opal_add_one_export(struct kobject *parent, const char *export_name,
struct device_node *np, const char *prop_name)
{
- struct bin_attribute *attr = NULL;
- const char *name = NULL;
+ struct bin_attribute *attr;
+ const char *name;
u64 vals[2];
int rc;
rc = of_property_read_u64_array(np, prop_name, &vals[0], 2);
if (rc)
- goto out;
+ return rc;
attr = kzalloc(sizeof(*attr), GFP_KERNEL);
name = kstrdup(export_name, GFP_KERNEL);
--
2.17.1
Here needs a NULL check, as kzalloc may fail returning NULL.
Issue was found by coccinelle.
Generated by: scripts/coccinelle/null/kmerr.cocci
Signed-off-by: Qiujun Huang <[email protected]>
Reviewed-by: Oliver O'Halloran <[email protected]>
---
arch/powerpc/platforms/powernv/opal.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 71af1cbc6334..908d749bcef5 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -811,6 +811,9 @@ static int opal_add_one_export(struct kobject *parent, const char *export_name,
return rc;
attr = kzalloc(sizeof(*attr), GFP_KERNEL);
+ if (!attr)
+ return -ENOMEM;
+
name = kstrdup(export_name, GFP_KERNEL);
if (!name) {
rc = -ENOMEM;
--
2.17.1
> And we can remove the redundant assignments to attr and name.
How do you think about a wording like the following?
Two local variables will eventually be set to appropriate pointers
a bit later. Thus omit their explicit initialisation at the beginning.
Regards,
Markus
> Here needs a NULL check, as kzalloc may fail returning NULL.
>
> Issue was found by coccinelle.
* Do you really try to ignore (my) specific patch review comments
(for a moment)?
https://lore.kernel.org/linuxppc-dev/[email protected]/
https://lore.kernel.org/patchwork/comment/1414845/
https://lkml.org/lkml/2020/4/6/279
* Would you like to integrate further adjustments with a varying delay?
(Are you waiting on nicer feedback by any software maintainers?)
Regards,
Markus
> We don't need to go to the labal of out …
Please avoid a typo for this change description.
> fails, as there is nothing to do. Just return.
I suggest to reconsider also this wording.
Return directly after a call of the function “of_property_read_u64_array”
failed at the beginning.
> And we can …
Do such words indicate that even this patch approach should be split
into further update steps?
Regards,
Markus