On Wed, Oct 3, 2018 at 1:25 PM Lubomir Rintel <lkundrak@xxxxx> wrote:
>
> On Wed, 2018-10-03 at 11:08 -0500, Rob Herring wrote:
> > On Wed, Oct 3, 2018 at 8:22 AM Lubomir Rintel <lkundrak@xxxxx> wrote:
> > > If none of the markers bears type information we can't just prepend a new
> > > one. Otherwise two subsequent markers will begin at offset 0, blowing up
> > > the chunk_len calculation a few lines after.
> > >
> > > A particular case where this causes havoc is a string property in a tree
> > > generated from "fs" input. A string property ends up with a TYPE_NULL
> >
> > You mean TYPE_NONE?
>
> Yes.
>
> > > marker. Upon output, in write_propval(), we correctly guess it looks
> > > like a TYPE_STRING. With the dummy marker prepended, We'd and up with two
> > > markers:
> > >
> > > { type = TYPE_STRING, offset = 0 }
> > > { type = TYPE_NONE, offset = 0 }
> > >
> > > The chunk_len for the fist marker will end up being calculated as 0.
> > > This blows up later on because a zero-length string should never happen
> > > because it lacks the terminating \0 (valgrind catches this).
> >
> > I'm slightly confused as to when this is broken. This sounds like the
> > output from fs input is completely broken for all strings, but I don't
> > think that is true given we've had reports of only minor issues.
>
> Yes, it's indeed broken for all strings. On my system:
>
> $ ./dtc -I fs -O dts tests/fs/test_tree1 |grep '"'
> compatible = """test_tree1";
> prop-str = """hello world";
> compatible = """subsubnode2\0subsubnode";
> compatible = """subnode1";
> compatible = """subsubnode1\0subsubnode";
> placeholder = """this is a placeholder string\0string2";
>
> Note the extra "". But also:
Ah, the extra "" should be fixed with the patch I mentioned. I guess
TYPE_NONE is the same as REF_PATH combined with TYPE_STRING.
> $ valgrind ./dtc -I fs -O dts tests/fs/test_tree1 |grep '"'
And valgrind passes for me with the patch.
Rob
