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: <20110223194510.GS14597@angua.secretlab.ca>
Date:	Wed, 23 Feb 2011 12:45:10 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Andres Salomon <dilinger@...ued.net>
Cc:	Daniel Drake <dsd@...top.org>,
	David Woodhouse <dwmw2@...radead.org>, cbou@...l.ru,
	linux-kernel@...r.kernel.org, x86@...nel.org, tglx@...utronix.de,
	mingo@...hat.com, hpa@...or.com, dmitry.torokhov@...il.com,
	devicetree-discuss@...ts.ozlabs.org,
	Mitch Bradley <wmb@...top.org>,
	"David S. Miller" <davem@...emloft.net>, sparclinux@...r.kernel.org
Subject: Re: [PATCH] of/pdt: don't bother parsing pkg2path results, return
 as-is

On Fri, Feb 18, 2011 at 07:12:25PM -0800, Andres Salomon wrote:
> On Fri, 18 Feb 2011 19:06:59 -0800
> Andres Salomon <dilinger@...ued.net> wrote:
> 
> > On Fri, 18 Feb 2011 23:42:57 +0000
> > Daniel Drake <dsd@...top.org> wrote:
> > 
> > > On 16 February 2011 22:44, David Woodhouse <dwmw2@...radead.org>
> > > wrote:
> > > > On Wed, 2011-02-16 at 22:28 +0000, Daniel Drake wrote:
> > > >>
> > > >> +static int __init add_common_platform_devices(void)
> > > >> +{
> > > >> +       struct platform_device *pdev;
> > > >> +
> > > >> +       pdev = platform_device_register_simple("olpc-battery",
> > > >> -1, NULL, 0);
> > > >> +       if (IS_ERR(pdev))
> > > >> +               return PTR_ERR(pdev);
> > > >> +
> > > >> +       return 0;
> > > >> +}
> > > >> +
> > > >
> > > > Still kind of sucks that you have to do this, and can't bind to
> > > > something in the device-tree.
> > > 
> > > OK, feel free to put this patch on hold for now. I started looking
> > > at the device tree approach today. It looks doable but first we
> > > have to fix a DT bug/inconsistency that is preventing us from
> > > correctly binding to the tree's devices.
> > > 
> > > Daniel
> > 
> > 
> > Mea culpa.  The patch below fixes a bug I introduced earlier.
> > Cc'ing the sparc folks, as this probably affects them
> > (although I would think that it fixes broken behavior for them..?)
> > 
> > 
> > 
> 
> And this is a followup to the prior patch; an optimization based
> upon what we're doing with pkg2path stuff.
> 
> The sparc folks don't use pkg2path, so this shouldn't affect them at
> all.
> 
> Cc'ing Mitch as well, in case there's some reason why I should be
> wary of doing this.  :)
> 
> 
> 
> 
> From: Andres Salomon <dilinger@...ued.net>
> 
> The of_pdt_try_pkg2path() function currently allocates memory for
> and fetches the full path name from OFW (of the form '/foo/bar/baz@0').
> It then returns only the name ('baz@0)', and the caller re-constructs
> the full name (for dp->full_name) based upon dp->parent->full_name and
> what was returned by of_pdt_try_pkg2path().  Oh, and it allocates more
> memory for it.
> 
> OLPC is the only architecture which fills in the pkg2path hook, so
> this shouldn't affect any other users (ie, sparc).  Since in practice
> (dp->parent->full_name + '/' + strrchr(pkg2path, '/')) and the result
> from pkg2path end up being exactly the same, just short circuit the rest
> of the of_pdt_build_full_name logic and set dp->full_name to the result
> of pkg2path.  This means we don't have to parse it nor allocate more
> memory for it.
> 
> This saves time, code, and memory.
> 
> Signed-off-by: Andres Salomon <dilinger@...ued.net>
> ---
>  drivers/of/pdt.c |   17 +++++++----------
>  1 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/of/pdt.c b/drivers/of/pdt.c
> index b39d584..605834b 100644
> --- a/drivers/of/pdt.c
> +++ b/drivers/of/pdt.c
> @@ -134,7 +134,7 @@ static char * __init of_pdt_get_one_property(phandle node, const char *name)
>  
>  static char * __init of_pdt_try_pkg2path(phandle node)
>  {
> -	char *res, *buf = NULL;
> +	char *buf = NULL;
>  	int len;
>  
>  	if (!of_pdt_prom_ops->pkg2path)
> @@ -147,13 +147,7 @@ static char * __init of_pdt_try_pkg2path(phandle node)
>  		pr_err("%s: package-to-path failed\n", __func__);
>  		return NULL;
>  	}
> -
> -	res = strrchr(buf, '/');
> -	if (!res) {
> -		pr_err("%s: couldn't find / in %s\n", __func__, buf);
> -		return NULL;
> -	}
> -	return res+1;
> +	return buf;
>  }
>  
>  static struct device_node * __init of_pdt_create_node(phandle node,
> @@ -193,10 +187,13 @@ static char * __init of_pdt_build_full_name(struct device_node *dp,
>  	 * name property (ie, 'battery'), we want the name we see with
>  	 * package-to-path (ie, 'battery@0').
>  	 */
> +	n = of_pdt_try_pkg2path(node);
> +	if (n)
> +		return n;
> +
> +	/* Older methods for determining full name */
>  	name = of_pdt_node_name(dp);
>  	if (!name)
> -		name = of_pdt_try_pkg2path(node);
> -	if (!name)

Ah, this addresses one of the comments I just made on the first patch.
I'd squash the two patches together before you repost.

g.

>  		name = dp->name;
>  
>  	plen = strlen(dp->parent->full_name);
> -- 
> 1.7.2.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ