Re: [PATCH 1/3] fdt: Add a function to count strings

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]

On Thu, Jul 16, 2015 at 01:13:53PM +0200, Thierry Reding wrote:
> On Wed, Jul 15, 2015 at 11:41:30PM +1000, David Gibson wrote:
> > On Wed, Jul 15, 2015 at 01:13:57PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > 
> > > Given a device tree node and a property name, the fdt_count_strings()
> > > function counts the number of strings found in the property value.
> > > 
> > > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  libfdt/fdt_ro.c      | 20 ++++++++++++++++
> > >  libfdt/libfdt.h      |  9 ++++++++
> > >  tests/.gitignore     |  1 +
> > >  tests/Makefile.tests |  1 +
> > >  tests/run_tests.sh   |  3 +++
> > >  tests/strings.c      | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/strings.dts    | 10 ++++++++
> > >  7 files changed, 108 insertions(+)
> > >  create mode 100644 tests/strings.c
> > >  create mode 100644 tests/strings.dts
> > > 
> > > diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
> > > index a65e4b5b72b6..874975a0d8ad 100644
> > > --- a/libfdt/fdt_ro.c
> > > +++ b/libfdt/fdt_ro.c
> > > @@ -538,6 +538,26 @@ int fdt_stringlist_contains(const char *strlist, int listlen, const char *str)
> > >  	return 0;
> > >  }
> > >  
> > > +int fdt_count_strings(const void *fdt, int nodeoffset, const char *property)
> > > +{
> > > +	int length, i, count = 0;
> > > +	const char *list;
> > > +
> > > +	list = fdt_getprop(fdt, nodeoffset, property, &length);
> > > +	if (!list)
> > > +		return -length;
> > > +
> > > +	for (i = 0; i < length; i++) {
> > > +		int len = strlen(list);
> > 
> > I like the concept of these patches, but this implementation is unsafe
> > if the given property does not, in fact, contain a list of \0
> > terminated strings.
> 
> This should be fixed in v2 of the patches. This isn't quite as simple as
> using strnlen() instead of strlen() because it also means we need extra
> handling for the case where a NUL byte isn't found within the value.

Yes, I suspect it will actually be easier to use memchr().

> There is also a bit of an inconsistency for the fdt_find_string() and
> fdt_get_string() functions because they can successfully work with non-
> NUL-terminated values. I've gone into some detail about that in the v2
> cover letter, so feedback there would be most welcome.
> 
> Thierry



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]