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

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

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

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.

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

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