On Sat, Mar 04, 2023 at 01:58:17PM -0600, Samuel Holland wrote:
> On 2/20/23 04:39, Michal Suchánek wrote:
> > On Sun, Feb 19, 2023 at 11:59:36PM -0600, Samuel Holland wrote:
> >> Do not return both NULL and error pointers. The function is only
> >> documented as returning error pointers.
> >>
> >> Fixes: 8a1661f20e6c ("drivers: clk: Handle gracefully NULL pointers")
> >> Signed-off-by: Samuel Holland <sam...@sholland.org>
> >> ---
> >>
> >> drivers/clk/clk-uclass.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> >> index 78299dbceb2..5bce976b060 100644
> >> --- a/drivers/clk/clk-uclass.c
> >> +++ b/drivers/clk/clk-uclass.c
> >> @@ -490,7 +490,7 @@ struct clk *clk_get_parent(struct clk *clk)
> >>
> >> debug("%s(clk=%p)\n", __func__, clk);
> >> if (!clk_valid(clk))
> >> - return NULL;
> >> + return ERR_PTR(-ENODEV);
> >>
> >> pdev = dev_get_parent(clk->dev);
> >> if (!pdev)
> >
> > Do we really need this?
> >
> > Who cares about the distinction?
> >
> > Just returning NULL on any error makes the code so much simpler and
> > easier to understand.
>
> I'm fine with returning only NULL, or only ERR_PTR(-ENODEV) as done
> later in the function, but returning both makes the error handling in
> callers more complicated for no benefit. It sounds like you would prefer
> I change the later ERR_PTR(-ENODEV) to NULL instead of this change? I
> can do that for v2.
I think NULL is easier to check, and so long as there is no meaningful
distinction between ERR_PTR and NULL we should converge towards NULL as
the single error return value.
Thanks
Michal
