lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_JsqLVnQ12AS-2CNfsf5PsyGvELrMjDDGfRcv7opijgJkGdw@mail.gmail.com>
Date:	Mon, 9 May 2016 16:47:34 -0500
From:	Rob Herring <robherring2@...il.com>
To:	Pantelis Antoniou <pantelis.antoniou@...sulko.com>
Cc:	Frank Rowand <frowand.list@...il.com>,
	Matt Porter <mporter@...sulko.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Koen Kooi <koen@...inion.thruhere.net>,
	Guenter Roeck <linux@...ck-us.net>,
	Marek Vasut <marex@...x.de>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Pantelis Antoniou <panto@...oniou-consulting.com>
Subject: Re: [PATCH 3/3] of: overlay: Pick up label symbols from overlays.

On Mon, May 9, 2016 at 1:11 PM, Pantelis Antoniou
<pantelis.antoniou@...sulko.com> wrote:
> Insert overlay symbols to the base tree when applied.
> This makes it possible to apply an overlay that references a label
> that a previously inserted overlay had.
>
> Signed-off-by: Pantelis Antoniou <pantelis.antoniou@...sulko.com>
> Tested-by: Geert Uytterhoeven <geert+renesas@...der.be>
>
> ---
>  drivers/of/overlay.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index a274d65..a7956a2 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -514,6 +514,101 @@ static int of_free_overlay_info(struct of_overlay *ov)
>         return 0;
>  }
>
> +static int of_overlay_add_symbols(
> +               struct device_node *tree,
> +               struct of_overlay *ov)
> +{
> +       struct of_overlay_info *ovinfo;
> +       struct device_node *root_sym = NULL;
> +       struct device_node *child = NULL;
> +       struct property *prop;
> +       const char *path, *s;
> +       char *new_path;
> +       int i, len, err;
> +
> +       /* this may fail (if no fixups are required) */
> +       root_sym = of_find_node_by_path("/__symbols__");
> +
> +       /* do nothing if no root symbols */
> +       if (!root_sym)
> +               return 0;
> +
> +       /* locate the symbols & fixups nodes on resolve */
> +       for_each_child_of_node(tree, child) {
> +               if (of_node_cmp(child->name, "__symbols__") == 0)
> +                       goto found;

Doesn't of_get_child_by_name work here?

> +       }
> +       /* no symbols, no problem */
> +       of_node_put(root_sym);
> +       return 0;
> +
> +found:

As Marek said, get rid of the goto.

> +       err = -EINVAL;
> +       for_each_property_of_node(child, prop) {
> +
> +               /* skip properties added automatically */
> +               if (of_prop_cmp(prop->name, "name") == 0)
> +                       continue;
> +
> +               err = of_property_read_string(child,
> +                               prop->name, &path);
> +               if (err != 0) {
> +                       pr_err("%s: Could not find symbol '%s'\n",
> +                                       __func__, prop->name);

Can you introduce and use a common pr_fmt prefix here and elsewhere
(just what you are adding).

> +                       continue;
> +               }
> +
> +

Extra blank line.

> +               /* now find fragment index */
> +               s = path;
> +
> +               /* compare paths to find fragment index */
> +               ovinfo = NULL;
> +               len = -1;

Move these into the for init.

> +               for (i = 0; i < ov->count; i++) {
> +                       ovinfo = &ov->ovinfo_tab[i];
> +
> +                       pr_debug("%s: #%d: overlay->name=%s target->name=%s\n",
> +                                       __func__, i, ovinfo->overlay->full_name,
> +                                       ovinfo->target->full_name);
> +
> +                       len = strlen(ovinfo->overlay->full_name);
> +                       if (strncasecmp(path, ovinfo->overlay->full_name,
> +                                               len) == 0 && path[len] == '/')
> +                               break;
> +               }
> +
> +               if (i >= ov->count)
> +                       continue;
> +
> +               pr_debug("%s: found target at #%d\n", __func__, i);
> +               new_path = kasprintf(GFP_KERNEL, "%s%s",
> +                               ovinfo->target->full_name,
> +                               path + len);
> +               if (!new_path) {
> +                       pr_err("%s: Failed to allocate propname for \"%s\"\n",
> +                                       __func__, prop->name);
> +                       continue;

If this fails, is there much point in continuing?

> +               }
> +
> +               err = of_changeset_add_property_string(&ov->cset, root_sym,
> +                               prop->name, new_path);
> +
> +               /* free always */
> +               kfree(new_path);
> +
> +               if (err) {

No need for brackets.

> +                       pr_err("%s: Failed to add property for \"%s\"\n",
> +                                       __func__, prop->name);
> +               }
> +       }
> +
> +       of_node_put(child);
> +       of_node_put(root_sym);
> +
> +       return 0;
> +}
> +
>  static LIST_HEAD(ov_list);
>  static DEFINE_IDR(ov_idr);
>
> @@ -642,6 +737,13 @@ static int __of_overlay_create(struct device_node *tree,
>                 goto err_abort_trans;
>         }
>
> +       err = of_overlay_add_symbols(tree, ov);
> +       if (err) {
> +               pr_err("%s: of_overlay_add_symbols() failed for tree@%s\n",
> +                               __func__, tree->full_name);
> +               goto err_abort_trans;
> +       }
> +
>         /* apply the changeset */
>         err = __of_changeset_apply(&ov->cset);
>         if (err) {
> --
> 1.7.12
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ