[PATCH v3 1/2] of: Fix issue where code would fall through to error case.

[PATCH v3 1/2] of: Fix issue where code would fall through to error case.

[Moritz Fischer]

No longer fall through into the error case that prints out
an error if no error (err = 0) occurred.

Rework error handling to print error where it occured instead
of having a global catch-all at the end of the function.

Fixes d9181b20a83(of: Add back an error message, restructured)
Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
---
Hi Rob, Frank

this has already Frank's Reviewed-by: tag on it, but since I changed around the
earlier part (before tree_node gets assigned) Frank might wanna take another look
at this.

Changes from v2:
* Change error printouts to print error where it's produced
* Before tree_symbols gets assigned a non-NULL value, directly return error
  instead of goto out, since of_put_node will be a no-op for !tree_symbols

Changes from  v1:
* Implemented Frank's suggestion

Cheers,

Moritz
---

 drivers/of/resolver.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 783bd09..5f51a4a 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -296,14 +296,12 @@ int of_resolve_phandles(struct device_node *overlay)
 	tree_symbols = NULL;
 
 	if (!overlay) {
-		pr_err("null overlay\n");
-		err = -EINVAL;
-		goto err_out;
+		pr_err("overlay phandle fixup failed: null overlay\n");
+		return -EINVAL;
 	}
 	if (!of_node_check_flag(overlay, OF_DETACHED)) {
-		pr_err("overlay not detached\n");
-		err = -EINVAL;
-		goto err_out;
+		pr_err("overlay phandle fixup failed: overlay not detached\n");
+		return -EINVAL;
 	}
 
 	phandle_delta = live_tree_max_phandle() + 1;
@@ -314,8 +312,10 @@ int of_resolve_phandles(struct device_node *overlay)
 			break;
 
 	err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta);
-	if (err)
-		goto err_out;
+	if (err) {
+		pr_err("overlay phandle fixup failed: %d\n", err);
+		return err;
+	}
 
 	overlay_fixups = NULL;
 
@@ -324,16 +324,13 @@ int of_resolve_phandles(struct device_node *overlay)
 			overlay_fixups = child;
 	}
 
-	if (!overlay_fixups) {
-		err = 0;
-		goto out;
-	}
+	if (!overlay_fixups)
+		return 0;
 
 	tree_symbols = of_find_node_by_path("/__symbols__");
 	if (!tree_symbols) {
-		pr_err("no symbols in root of device tree.\n");
-		err = -EINVAL;
-		goto err_out;
+		pr_err("overlay phandle fixup failed: no symbols in root\n");
+		return -EINVAL;
 	}
 
 	for_each_property_of_node(overlay_fixups, prop) {
@@ -344,13 +341,16 @@ int of_resolve_phandles(struct device_node *overlay)
 
 		err = of_property_read_string(tree_symbols,
 				prop->name, &refpath);
-		if (err)
-			goto err_out;
+		if (err) {
+			pr_err("overlay phandle fixup failed: %d\n", err);
+			goto out;
+		}
 
 		refnode = of_find_node_by_path(refpath);
 		if (!refnode) {
 			err = -ENOENT;
-			goto err_out;
+			pr_err("overlay phandle fixup failed: !refnode\n");
+			goto out;
 		}
 
 		phandle = refnode->phandle;
@@ -361,8 +361,6 @@ int of_resolve_phandles(struct device_node *overlay)
 			break;
 	}
 
-err_out:
-	pr_err("overlay phandle fixup failed: %d\n", err);
 out:
 	of_node_put(tree_symbols);
 
-- 
2.7.4

[PATCH v3 2/2] of: resolver: Fix checkpatch warnings

[Moritz Fischer]

Fix two line over 80 character warnings that checkpatch spit out:

Before:
total: 0 errors, 2 warnings, 374 lines checked
drivers/of/resolver.c has style problems, please review.

After:
total: 0 errors, 0 warnings, 376 lines checked

Signed-off-by: Moritz Fischer <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org>
---
Hi,

this one just silences two checkpatch warnings that I ran
into when running checkpatch against my patches.

There's a bunch of 'CHECK' level things in this file that
I could address in a follow up patch if desired.

Cheers,

Moritz
---

 drivers/of/resolver.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 5f51a4a..71f98dc 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -311,7 +311,8 @@ int of_resolve_phandles(struct device_node *overlay)
 		if (!of_node_cmp(local_fixups->name, "__local_fixups__"))
 			break;
 
-	err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta);
+	err = adjust_local_phandle_references(local_fixups, overlay,
+					      phandle_delta);
 	if (err) {
 		pr_err("overlay phandle fixup failed: %d\n", err);
 		return err;
@@ -356,7 +357,8 @@ int of_resolve_phandles(struct device_node *overlay)
 		phandle = refnode->phandle;
 		of_node_put(refnode);
 
-		err = update_usages_of_a_phandle_reference(overlay, prop, phandle);
+		err = update_usages_of_a_phandle_reference(overlay, prop,
+							   phandle);
 		if (err)
 			break;
 	}
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/2] of: resolver: Fix checkpatch warnings

[Moritz Fischer]

Fix two line over 80 character warnings that checkpatch spit out:

Before:
total: 0 errors, 2 warnings, 374 lines checked
drivers/of/resolver.c has style problems, please review.

After:
total: 0 errors, 0 warnings, 376 lines checked

Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
---
Hi,

this one just silences two checkpatch warnings that I ran
into when running checkpatch against my patches.

There's a bunch of 'CHECK' level things in this file that
I could address in a follow up patch if desired.

Cheers,

Moritz
---

 drivers/of/resolver.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 5f51a4a..71f98dc 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -311,7 +311,8 @@ int of_resolve_phandles(struct device_node *overlay)
 		if (!of_node_cmp(local_fixups->name, "__local_fixups__"))
 			break;
 
-	err = adjust_local_phandle_references(local_fixups, overlay, phandle_delta);
+	err = adjust_local_phandle_references(local_fixups, overlay,
+					      phandle_delta);
 	if (err) {
 		pr_err("overlay phandle fixup failed: %d\n", err);
 		return err;
@@ -356,7 +357,8 @@ int of_resolve_phandles(struct device_node *overlay)
 		phandle = refnode->phandle;
 		of_node_put(refnode);
 
-		err = update_usages_of_a_phandle_reference(overlay, prop, phandle);
+		err = update_usages_of_a_phandle_reference(overlay, prop,
+							   phandle);
 		if (err)
 			break;
 	}
-- 
2.7.4

Re: [PATCH v3 1/2] of: Fix issue where code would fall through to error case.

[Frank Rowand]

On 11/27/16 12:20, Moritz Fischer wrote:
> No longer fall through into the error case that prints out
> an error if no error (err = 0) occurred.
> 
> Rework error handling to print error where it occured instead
> of having a global catch-all at the end of the function.
> 
> Fixes d9181b20a83(of: Add back an error message, restructured)
> Signed-off-by: Moritz Fischer <moritz.fischer-+aYTwkv1SeIAvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Frank Rowand <frank.rowand-mEdOJwZ7QcZBDgjK7y7TUQ@public.gmane.org>
> ---
> Hi Rob, Frank
> 
> this has already Frank's Reviewed-by: tag on it, but since I changed around the
> earlier part (before tree_node gets assigned) Frank might wanna take another look
> at this.

< snip >

Hi Moritz,

I agree with Rob's suggested one line solution (that I initially misunderstood).
The one line fix is to add "if (err)" immediately following label "err_out" in
of_resolve_phandles().

As far as patch 2/2, I'm not bothered by the two instances of line over 80 chars.
But if Rob wants to take patch 2/2 I have no objection.

-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/2] of: Fix issue where code would fall through to error case.

[Frank Rowand]

On 11/27/16 12:20, Moritz Fischer wrote:
> No longer fall through into the error case that prints out
> an error if no error (err = 0) occurred.
> 
> Rework error handling to print error where it occured instead
> of having a global catch-all at the end of the function.
> 
> Fixes d9181b20a83(of: Add back an error message, restructured)
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
> ---
> Hi Rob, Frank
> 
> this has already Frank's Reviewed-by: tag on it, but since I changed around the
> earlier part (before tree_node gets assigned) Frank might wanna take another look
> at this.

< snip >

Hi Moritz,

I agree with Rob's suggested one line solution (that I initially misunderstood).
The one line fix is to add "if (err)" immediately following label "err_out" in
of_resolve_phandles().

As far as patch 2/2, I'm not bothered by the two instances of line over 80 chars.
But if Rob wants to take patch 2/2 I have no objection.

-Frank