Messages in this thread | | | Subject | Re: [PATCH v4 1/2] clk: hisilicon: Use correct return value about hisi_reset_init() | From | Tiezhu Yang <> | Date | Fri, 29 May 2020 12:02:05 +0800 |
| |
On 05/29/2020 11:58 AM, Stephen Boyd wrote: > Quoting Tiezhu Yang (2020-05-28 19:03:54) >> On 05/29/2020 07:15 AM, Stephen Boyd wrote: >>> Quoting Tiezhu Yang (2020-05-27 19:27:42) >>>> On 05/28/2020 03:06 AM, Stephen Boyd wrote: >>>>> Quoting Tiezhu Yang (2020-05-27 07:39:21) >>>>>> The return value about hisi_reset_init() is not correct, fix it. >>>>>> >>>>>> Fixes: e9a2310fb689 ("reset: hisilicon: fix potential NULL pointer dereference") >>>>> hisi_reset_init() returns NULL on error in that commit. This patch >>>>> doesn't make sense. >>>> Hi Stephen, >>>> >>>> The initial aim of this patch is to use correct return value about >>>> hisi_reset_init(), maybe NULL is OK, but the return value in this >>>> patch is more accurate. >>> The implementation of hisi_reset_init() that I see is this: >>> >>> >>> struct hisi_reset_controller *rstc; >>> >>> rstc = devm_kmalloc(&pdev->dev, sizeof(*rstc), GFP_KERNEL); >>> if (!rstc) >>> return NULL; >>> >>> rstc->membase = devm_platform_ioremap_resource(pdev, 0); >>> if (IS_ERR(rstc->membase)) >>> return NULL; >>> >>> spin_lock_init(&rstc->lock); >>> rstc->rcdev.owner = THIS_MODULE; >>> rstc->rcdev.ops = &hisi_reset_ops; >>> rstc->rcdev.of_node = pdev->dev.of_node; >>> rstc->rcdev.of_reset_n_cells = 2; >>> rstc->rcdev.of_xlate = hisi_reset_of_xlate; >>> reset_controller_register(&rstc->rcdev); >>> >>> return rstc; >>> >>> And that returns NULL on an error and a valid pointer on success. >>> Changing the code to check the return value of hisi_reset_init() for an >>> error pointer is simply wrong without updating hisi_reset_init() to >>> return an error pointer on error. Where is the patch that changes >>> hisi_reset_init() to return an error pointer? >> Hi Stephen, >> >> Do you mean the following changes? > Yes where is this change?
ERR_PTR(-ENOMEM) and ERR_CAST(rstc->membase)
rstc = devm_kmalloc(&pdev->dev, sizeof(*rstc), GFP_KERNEL); if (!rstc) - return NULL; + return ERR_PTR(-ENOMEM); rstc->membase = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(rstc->membase)) - return NULL; + return ERR_CAST(rstc->membase);
| |