* [PATCH] of: overlay: Remove else after goto
@ 2017-11-28 8:26 Geert Uytterhoeven
[not found] ` <1511857593-18574-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Geert Uytterhoeven @ 2017-11-28 8:26 UTC (permalink / raw)
To: Rob Herring, Frank Rowand, Pantelis Antoniou
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven
If an "if" branch is terminated by a "goto", there's no need to have an
"else" statement and an indented block of code.
Remove the "else" statement to simplify the code flow for the casual
reviewer.
Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
drivers/of/overlay.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 8676411bd3e6f29f..53bc9e3f0b985415 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -580,9 +580,9 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
of_node_put(fragment->overlay);
ret = -EINVAL;
goto err_free_fragments;
- } else {
- cnt++;
}
+
+ cnt++;
}
}
@@ -736,14 +736,12 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
devicetree_state_flags |= DTSF_APPLY_FAIL;
}
goto err_free_overlay_changeset;
- } else {
- ret = __of_changeset_apply_notify(&ovcs->cset);
- if (ret)
- pr_err("overlay changeset entry notify error %d\n",
- ret);
- /* fall through */
}
+ ret = __of_changeset_apply_notify(&ovcs->cset);
+ if (ret)
+ pr_err("overlay changeset entry notify error %d\n", ret);
+
list_add_tail(&ovcs->ovcs_list, &ovcs_list);
*ovcs_id = ovcs->id;
@@ -931,15 +929,12 @@ int of_overlay_remove(int *ovcs_id)
if (ret_apply)
devicetree_state_flags |= DTSF_REVERT_FAIL;
goto out_unlock;
- } else {
- ret = __of_changeset_revert_notify(&ovcs->cset);
- if (ret) {
- pr_err("overlay changeset entry notify error %d\n",
- ret);
- /* fall through - changeset was reverted */
- }
}
+ ret = __of_changeset_revert_notify(&ovcs->cset);
+ if (ret)
+ pr_err("overlay changeset entry notify error %d\n", ret);
+
*ovcs_id = 0;
ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE);
--
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] of: overlay: Remove else after goto
[not found] ` <1511857593-18574-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
@ 2017-11-28 15:28 ` Rob Herring
2017-11-29 9:30 ` Frank Rowand
1 sibling, 0 replies; 5+ messages in thread
From: Rob Herring @ 2017-11-28 15:28 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Frank Rowand, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA
On Tue, Nov 28, 2017 at 09:26:33AM +0100, Geert Uytterhoeven wrote:
> If an "if" branch is terminated by a "goto", there's no need to have an
> "else" statement and an indented block of code.
>
> Remove the "else" statement to simplify the code flow for the casual
> reviewer.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
> drivers/of/overlay.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
Applied.
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] of: overlay: Remove else after goto
[not found] ` <1511857593-18574-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
2017-11-28 15:28 ` Rob Herring
@ 2017-11-29 9:30 ` Frank Rowand
[not found] ` <6d0dc834-6df6-fa6c-3adc-5506f432fa7d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
1 sibling, 1 reply; 5+ messages in thread
From: Frank Rowand @ 2017-11-29 9:30 UTC (permalink / raw)
To: Geert Uytterhoeven, Rob Herring, Pantelis Antoniou
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA
On 11/28/17 03:26, Geert Uytterhoeven wrote:
> If an "if" branch is terminated by a "goto", there's no need to have an
> "else" statement and an indented block of code.
>
> Remove the "else" statement to simplify the code flow for the casual
> reviewer.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> ---
> drivers/of/overlay.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 8676411bd3e6f29f..53bc9e3f0b985415 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -580,9 +580,9 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
> of_node_put(fragment->overlay);
> ret = -EINVAL;
> goto err_free_fragments;
> - } else {
> - cnt++;
> }
> +
> + cnt++;
> }
> }
>
> @@ -736,14 +736,12 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
> devicetree_state_flags |= DTSF_APPLY_FAIL;
> }
> goto err_free_overlay_changeset;
> - } else {
> - ret = __of_changeset_apply_notify(&ovcs->cset);
> - if (ret)
> - pr_err("overlay changeset entry notify error %d\n",
> - ret);
> - /* fall through */
> }
>
> + ret = __of_changeset_apply_notify(&ovcs->cset);
> + if (ret)
> + pr_err("overlay changeset entry notify error %d\n", ret);
> +
The fall through comment was removed. It is important to document that we
are intentionally continuing despite the error.
> list_add_tail(&ovcs->ovcs_list, &ovcs_list);
> *ovcs_id = ovcs->id;
>
> @@ -931,15 +929,12 @@ int of_overlay_remove(int *ovcs_id)
> if (ret_apply)
> devicetree_state_flags |= DTSF_REVERT_FAIL;
> goto out_unlock;
> - } else {
> - ret = __of_changeset_revert_notify(&ovcs->cset);
> - if (ret) {
> - pr_err("overlay changeset entry notify error %d\n",
> - ret);
> - /* fall through - changeset was reverted */
> - }
> }
>
> + ret = __of_changeset_revert_notify(&ovcs->cset);
> + if (ret)
> + pr_err("overlay changeset entry notify error %d\n", ret);
> +
And same here.
> *ovcs_id = 0;
>
> ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE);
>
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] of: overlay: Remove else after goto
[not found] ` <6d0dc834-6df6-fa6c-3adc-5506f432fa7d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2017-11-30 15:34 ` Rob Herring
[not found] ` <CAL_JsqLjAJ0QG5r217bpT4ZLaKSras6taWHjooTsdL3GDp9_hg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Rob Herring @ 2017-11-30 15:34 UTC (permalink / raw)
To: Frank Rowand
Cc: Geert Uytterhoeven, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA
On Wed, Nov 29, 2017 at 3:30 AM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 11/28/17 03:26, Geert Uytterhoeven wrote:
>> If an "if" branch is terminated by a "goto", there's no need to have an
>> "else" statement and an indented block of code.
>>
>> Remove the "else" statement to simplify the code flow for the casual
>> reviewer.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>> ---
>> drivers/of/overlay.c | 25 ++++++++++---------------
>> 1 file changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index 8676411bd3e6f29f..53bc9e3f0b985415 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -580,9 +580,9 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
>> of_node_put(fragment->overlay);
>> ret = -EINVAL;
>> goto err_free_fragments;
>> - } else {
>> - cnt++;
>> }
>> +
>> + cnt++;
>> }
>> }
>>
>> @@ -736,14 +736,12 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
>> devicetree_state_flags |= DTSF_APPLY_FAIL;
>> }
>> goto err_free_overlay_changeset;
>> - } else {
>> - ret = __of_changeset_apply_notify(&ovcs->cset);
>> - if (ret)
>> - pr_err("overlay changeset entry notify error %d\n",
>> - ret);
>> - /* fall through */
>> }
>>
>> + ret = __of_changeset_apply_notify(&ovcs->cset);
>> + if (ret)
>> + pr_err("overlay changeset entry notify error %d\n", ret);
>> +
>
> The fall through comment was removed. It is important to document that we
> are intentionally continuing despite the error.
I've fixed these up like this:
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 53bc9e3f0b98..cb520169d679 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -741,6 +741,7 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
ret = __of_changeset_apply_notify(&ovcs->cset);
if (ret)
pr_err("overlay changeset entry notify error %d\n", ret);
+ /* notify failure is not fatal, continue */
list_add_tail(&ovcs->ovcs_list, &ovcs_list);
*ovcs_id = ovcs->id;
@@ -934,6 +935,7 @@ int of_overlay_remove(int *ovcs_id)
ret = __of_changeset_revert_notify(&ovcs->cset);
if (ret)
pr_err("overlay changeset entry notify error %d\n", ret);
+ /* notify failure is not fatal, continue */
*ovcs_id = 0;
--
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] of: overlay: Remove else after goto
[not found] ` <CAL_JsqLjAJ0QG5r217bpT4ZLaKSras6taWHjooTsdL3GDp9_hg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2017-11-30 21:52 ` Frank Rowand
0 siblings, 0 replies; 5+ messages in thread
From: Frank Rowand @ 2017-11-30 21:52 UTC (permalink / raw)
To: Rob Herring
Cc: Geert Uytterhoeven, Pantelis Antoniou, devicetree-u79uwXL29TY76Z2rM5mHXA
On 11/30/17 10:34, Rob Herring wrote:
> On Wed, Nov 29, 2017 at 3:30 AM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 11/28/17 03:26, Geert Uytterhoeven wrote:
>>> If an "if" branch is terminated by a "goto", there's no need to have an
>>> "else" statement and an indented block of code.
>>>
>>> Remove the "else" statement to simplify the code flow for the casual
>>> reviewer.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>>> ---
>>> drivers/of/overlay.c | 25 ++++++++++---------------
>>> 1 file changed, 10 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>> index 8676411bd3e6f29f..53bc9e3f0b985415 100644
>>> --- a/drivers/of/overlay.c
>>> +++ b/drivers/of/overlay.c
>>> @@ -580,9 +580,9 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
>>> of_node_put(fragment->overlay);
>>> ret = -EINVAL;
>>> goto err_free_fragments;
>>> - } else {
>>> - cnt++;
>>> }
>>> +
>>> + cnt++;
>>> }
>>> }
>>>
>>> @@ -736,14 +736,12 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
>>> devicetree_state_flags |= DTSF_APPLY_FAIL;
>>> }
>>> goto err_free_overlay_changeset;
>>> - } else {
>>> - ret = __of_changeset_apply_notify(&ovcs->cset);
>>> - if (ret)
>>> - pr_err("overlay changeset entry notify error %d\n",
>>> - ret);
>>> - /* fall through */
>>> }
>>>
>>> + ret = __of_changeset_apply_notify(&ovcs->cset);
>>> + if (ret)
>>> + pr_err("overlay changeset entry notify error %d\n", ret);
>>> +
>>
>> The fall through comment was removed. It is important to document that we
>> are intentionally continuing despite the error.
>
> I've fixed these up like this:
Thanks, looks good.
-Frank
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index 53bc9e3f0b98..cb520169d679 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -741,6 +741,7 @@ int of_overlay_apply(struct device_node *tree, int *ovcs_id)
> ret = __of_changeset_apply_notify(&ovcs->cset);
> if (ret)
> pr_err("overlay changeset entry notify error %d\n", ret);
> + /* notify failure is not fatal, continue */
>
> list_add_tail(&ovcs->ovcs_list, &ovcs_list);
> *ovcs_id = ovcs->id;
> @@ -934,6 +935,7 @@ int of_overlay_remove(int *ovcs_id)
> ret = __of_changeset_revert_notify(&ovcs->cset);
> if (ret)
> pr_err("overlay changeset entry notify error %d\n", ret);
> + /* notify failure is not fatal, continue */
>
> *ovcs_id = 0;
>
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-11-30 21:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-28 8:26 [PATCH] of: overlay: Remove else after goto Geert Uytterhoeven
[not found] ` <1511857593-18574-1-git-send-email-geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
2017-11-28 15:28 ` Rob Herring
2017-11-29 9:30 ` Frank Rowand
[not found] ` <6d0dc834-6df6-fa6c-3adc-5506f432fa7d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-11-30 15:34 ` Rob Herring
[not found] ` <CAL_JsqLjAJ0QG5r217bpT4ZLaKSras6taWHjooTsdL3GDp9_hg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-11-30 21:52 ` Frank Rowand
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.