On Thu, Nov 17, 2016 at 6:10 PM, Moritz Fischer <moritz.fischer.private@xxxxxxxxx> wrote: > On Thu, Nov 17, 2016 at 4:02 PM, Frank Rowand <frowand.list@xxxxxxxxx> 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 <moritz.fischer@xxxxxxxxx> >>>> --- >>>> 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 <frank.rowand@xxxxxxxxxxx> >>> >>> -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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html