On Thu, Nov 17, 2016 at 6:10 PM, Moritz Fischer <[email protected]> wrote: > On Thu, Nov 17, 2016 at 4:02 PM, Frank Rowand <[email protected]> wrote: >> On 11/17/16 15:40, Frank Rowand wrote: >>> On 11/17/16 15:25, Moritz Fischer wrote: >>>> No longer fall through into the error case that prints out >>>> an error if no error (err = 0) occurred. >>>> >>>> Fixes d9181b20a83(of: Add back an error message, restructured) >>>> Signed-off-by: Moritz Fischer <[email protected]> >>>> --- >>>> drivers/of/resolver.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c >>>> index 783bd09..785076d 100644 >>>> --- a/drivers/of/resolver.c >>>> +++ b/drivers/of/resolver.c >>>> @@ -358,9 +358,13 @@ int of_resolve_phandles(struct device_node *overlay) >>>> >>>> err = update_usages_of_a_phandle_reference(overlay, prop, >>>> phandle); >>>> if (err) >>>> - break; >>>> + goto err_out; >>>> } >>>> >>>> + of_node_put(tree_symbols); >>>> + >>>> + return 0; >>>> + >>>> err_out: >>>> pr_err("overlay phandle fixup failed: %d\n", err); >>>> out: >>> >>> Thanks for catching that. >>> >>> Rob, please apply. >>> >>> Reviewed-by: Frank Rowand <[email protected]> >>> >>> -Frank >> >> On second thought, isn't the common pattern when clean up is needed for >> both the no-error path and the error path something like: >> >> >> out: >> of_node_put(tree_symbols); >> return err; >> >> err_out: >> pr_err("overlay phandle fixup failed: %d\n", err); >> goto out; >> } >> >> >> I don't have a strong opinion, whatever Rob wants to take is fine with me. > > Same here. I tried to avoid the jumping back part, but if that's the > common pattern, > I can submit a v2 doing that instead.
Both are ugly. Just do: if (err) pr_err(...); Rob
