2016-11-27 20:21:20

by Moritz Fischer

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

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 <[email protected]>
Reviewed-by: Frank Rowand <[email protected]>
---
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


2016-11-27 20:21:32

by Moritz Fischer

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

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 <[email protected]>
---
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

2016-11-29 18:41:35

by Frank Rowand

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

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 <[email protected]>
> Reviewed-by: Frank Rowand <[email protected]>
> ---
> 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