On Fri, 2018-10-12 at 21:53 -0700, frowand.l...@gmail.com wrote:
> From: Frank Rowand <frank.row...@sony.com>
>
> Multiple overlay fragments adding or deleting the same node is not
> supported. Replace code comment of such, with check to detect the
> attempt and fail the overlay apply.
>
> Devicetree unittest where multiple fragments added the same node was
> added in the previous patch in the series. After applying this patch
> the unittest messages will no longer include:
>
> Duplicate name in motor-1, renamed to "controller#1"
> OF: overlay: of_overlay_apply() err=0
> ### dt-test ### of_overlay_fdt_apply() expected -22, ret=0,
> overlay_bad_add_dup_node
> ### dt-test ### FAIL of_unittest_overlay_high_level():2419 Adding overlay
> 'overlay_bad_add_dup_node' failed
>
> ...
>
> ### dt-test ### end of unittest - 210 passed, 1 failed
>
> but will instead include:
>
> OF: overlay: ERROR: multiple overlay fragments add and/or delete node
> /testcase-data-2/substation@100/motor-1/controller
>
> ...
>
> ### dt-test ### end of unittest - 211 passed, 0 failed
[]
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
[]
> @@ -523,6 +515,54 @@ static int build_changeset_symbols_node(struct
> overlay_changeset *ovcs,
> }
>
> /**
> + * check_changeset_dup_add_node() - changeset validation: duplicate add node
> + * @ovcs: Overlay changeset
> + *
> + * Check changeset @ovcs->cset for multiple add node entries for the same
> + * node.
> + *
> + * Returns 0 on success, -ENOMEM if memory allocation failure, or -EINVAL if
> + * invalid overlay in @ovcs->fragments[].
> + */
> +static int check_changeset_dup_add_node(struct overlay_changeset *ovcs)
> +{
> + struct of_changeset_entry *ce_1, *ce_2;
> + char *fn_1, *fn_2;
> + int name_match;
> +
> + list_for_each_entry(ce_1, &ovcs->cset.entries, node) {
> +
> + if (ce_1->action == OF_RECONFIG_ATTACH_NODE ||
> + ce_1->action == OF_RECONFIG_DETACH_NODE) {
> +
> + ce_2 = ce_1;
> + list_for_each_entry_continue(ce_2, &ovcs->cset.entries,
> node) {
> + if (ce_2->action == OF_RECONFIG_ATTACH_NODE ||
> + ce_2->action == OF_RECONFIG_DETACH_NODE) {
> + /* inexpensive name compare */
> + if (!of_node_cmp(ce_1->np->full_name,
> + ce_2->np->full_name)) {
A bit of odd indentation here.
This line is normally aligned to the second ( on the line above.
> + /* expensive full path name
> compare */
> + fn_1 = kasprintf(GFP_KERNEL,
> "%pOF", ce_1->np);
> + fn_2 = kasprintf(GFP_KERNEL,
> "%pOF", ce_2->np);
> + name_match = !strcmp(fn_1,
> fn_2);
> + kfree(fn_1);
> + kfree(fn_2);
> + if (name_match) {
> + pr_err("ERROR: multiple
> overlay fragments add and/or delete node %pOF\n",
> + ce_1->np);
> + return -EINVAL;
> + }
> + }
> + }
> + }
> + }
> + }
> +
> + return 0;
> +}
Style trivia:
Using inverted tests and continue would reduce indentation.
list_for_each_entry(ce_1, &ovcs->cset.entries, node) {
if (ce_1->action != OF_RECONFIG_ATTACH_NODE &&
ce_1->action != OF_RECONFIG_DETACH_NODE)
continue;
ce_2 = ce_1;
list_for_each_entry_continue(ce_2, &ovcs->cset.entries, node) {
if (ce_2->action != OF_RECONFIG_ATTACH_NODE &&
ce_2->action != OF_RECONFIG_DETACH_NODE)
continue;
/* inexpensive name compare */
if (of_node_cmp(ce_1->np->full_name,
ce_2->np->full_name))
continue;
/* expensive full path name compare */
fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np);
fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np);
name_match = !strcmp(fn_1, fn_2);
kfree(fn_1);
kfree(fn_2);
if (name_match) {
pr_err("ERROR: multiple overlay fragments add
and/or delete node %pOF\n",
ce_1->np);
return -EINVAL;
}
}
}
