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]
Date:	Mon, 23 Nov 2015 16:53:24 -0800
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:	linux-clk@...r.kernel.org,
	Michael Turquette <mturquette@...libre.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Ben Dooks <ben.dooks@...ethink.co.uk>
Subject: Re: [PATCH 2/3] clk: let of_clk_get_parent_name() fail for invalid
 clock-indices

On 11/22, Masahiro Yamada wrote:
> 2015-11-21 2:45 GMT+09:00 Stephen Boyd <sboyd@...eaurora.org>:
> >
> > What is np pointing at? Something like:
> >
> >         consumer {
> >                 clocks = <&oscillator 0>;
> >         };
> >
> > Which would be invalid DT because oscillator doesn't have an
> > output for index 0?
> 
> 
> You are right.  My example was confusing.
> 
> 
> 
>          oscillator: oscillator {
>                  compatible = "myclocktype";
>                  #clock-cells = <1>;
>                  clock-indices = <1>, <3>;
>                  clock-output-names = "clka", "clkb";
>          };
> 
>          consumer {
>                  compatible = "myclockconsumer";
>                  clocks = <&oscillator 0>;
>          };
> 
> Currently, of_clk_get_parent_name(consumer_np, 0) returns "clks", but
> should return NULL;
> 
> I will rephrase the git-log in v2.
> 

Ok. Thanks.

> > Why can't we leave everything in place and check count == len at
> > the end? i.e.
> >
> >         of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
> >                 if (index == pv) {
> >                         index = count;
> >                         break;
> >                 }
> >                 count++;
> >         }
> >
> >         if (count == of_property_count_u32_elems(clkspec.np, "clock-indices"))
> >                 return NULL
> >
> 
> 
> Of course we can, although we have to mention "clock-indices" twice.
> 
> A good thing for of_get_property() is that we can get both the value
> and the length
> at the same time.
> 

Ok. Well if we don't want to count them again, perhaps a goto
jump over an unconditional return NULL would be better?

	of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
		if (index == pv) {
			index = count;
			goto found;
		}
		count++;
	}

	return NULL;
found:

Or since the macro for of_property_for_each_u32() tests the vp
poitner for NULL, we can check that pointer too...

	of_property_for_each_u32(clkspec.np, "clock-indices", prop, vp, pv) {
		if (index == pv) {
			index = count;
			break;
		}
		count++;
	}

	/* We didn't find anything */
	if (!vp)
		return NULL;

I guess I prefer the latter approach here.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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