On Thu, Jul 16, 2015 at 10:29:35PM +1000, David Gibson wrote:
> 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@xxxxxxxxxx>
> > > >
> > > > 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@xxxxxxxxxx>
> > > > ---
> > > > 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().
I don't see how that would make it easier. The problematic bit isn't
about determining whether or not the NUL byte is there. The problematic
bit is determining what to do about it. For example we could enforce
that these functions only work on well-defined properties, that is,
properties that end with a NUL-byte. Currently we don't do that in
fdt_find_string() and fdt_get_string() for performance reasons. For
fdt_count_strings() the check is implicit because the last string would
not be bounded by the property value if it wasn't terminated with a NUL
byte.
It would be easy to add this additional check as a way to short-circuit
implementations and abort early on malformed property values, but that
would be additional work that most users may not care about.
Thierry
Attachment:
signature.asc
Description: PGP signature
