[PATCH 2/3] dtoc: Fix widening an int array to an int

[Simon Glass]

Hi Walter,

On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wlozano at collabora.com> wrote:
>
> Hi Simon,
>
> I know you already merged this one but I have some doubts I would like
> to check with you.
>
> On 7/28/21 10:23 PM, Simon Glass wrote:
> > An int array can hold a single int so we should not need to do anything
> > in the widening operation. However due to a quirk in the code, an int[3]
> > widened with an int produced an int[4]. Fix this and add a test.
> >
> > Fix a comment typo while we are here.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > Reported-by: Tom Rini <trini at konsulko.com>
> > ---
> >
> >   test/dm/of_platdata.c   |  4 +---
> >   tools/dtoc/fdt.py       | 15 ++++++++-------
> >   tools/dtoc/test_dtoc.py |  6 +++---
> >   tools/dtoc/test_fdt.py  | 11 ++++++++++-
> >   4 files changed, 22 insertions(+), 14 deletions(-)
> >
> > diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
> > index 0f89c7a7da8..e3fa01afddf 100644
> > --- a/test/dm/of_platdata.c
> > +++ b/test/dm/of_platdata.c
> > @@ -35,11 +35,10 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
> >       plat = dev_get_plat(dev);
> >       ut_assert(plat->boolval);
> >       ut_asserteq(1, plat->intval);
> > -     ut_asserteq(4, ARRAY_SIZE(plat->intarray));
> > +     ut_asserteq(3, ARRAY_SIZE(plat->intarray));
> >       ut_asserteq(2, plat->intarray[0]);
> >       ut_asserteq(3, plat->intarray[1]);
> >       ut_asserteq(4, plat->intarray[2]);
> > -     ut_asserteq(0, plat->intarray[3]);
> >       ut_asserteq(5, plat->byteval);
> >       ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
> >       ut_asserteq(6, plat->bytearray[0]);
> > @@ -61,7 +60,6 @@ static int dm_test_of_plat_props(struct unit_test_state *uts)
> >       ut_asserteq(5, plat->intarray[0]);
> >       ut_asserteq(0, plat->intarray[1]);
> >       ut_asserteq(0, plat->intarray[2]);
> > -     ut_asserteq(0, plat->intarray[3]);
> >       ut_asserteq(8, plat->byteval);
> >       ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
> >       ut_asserteq(1, plat->bytearray[0]);
> > diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
> > index 9749966d5fb..429e95f9a96 100644
> > --- a/tools/dtoc/fdt.py
> > +++ b/tools/dtoc/fdt.py
> > @@ -163,13 +163,14 @@ class Prop:
> >                   self.value = new_value
> >               self.type = newprop.type
> >
> > -        if type(newprop.value) == list and type(self.value) != list:
> > -            self.value = [self.value]
> > -
> > -        if type(self.value) == list and len(newprop.value) > len(self.value):
> > -            val = self.GetEmpty(self.type)
> > -            while len(self.value) < len(newprop.value):
> > -                self.value.append(val)
> > +        if type(newprop.value) == list:
> > +            if type(self.value) != list:
> > +                self.value = [self.value]
> > +
> > +            if len(newprop.value) > len(self.value):
> > +                val = self.GetEmpty(self.type)
> > +                while len(self.value) < len(newprop.value):
> > +                    self.value.append(val)
> >
> >       @classmethod
> >       def GetEmpty(self, type):
> > diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
> > index 863ede90b7a..44d5d0c354a 100755
> > --- a/tools/dtoc/test_dtoc.py
> > +++ b/tools/dtoc/test_dtoc.py
> > @@ -296,7 +296,7 @@ struct dtd_sandbox_spl_test {
> >   \tbool\t\tboolval;
> >   \tunsigned char\tbytearray[3];
> >   \tunsigned char\tbyteval;
> > -\tfdt32_t\t\tintarray[4];
> > +\tfdt32_t\t\tintarray[3];
> >   \tfdt32_t\t\tintval;
> >   \tunsigned char\tlongbytearray[9];
> >   \tunsigned char\tnotstring[5];
> > @@ -354,7 +354,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test = {
> >   \t.boolval\t\t= true,
> >   \t.bytearray\t\t= {0x6, 0x0, 0x0},
> >   \t.byteval\t\t= 0x5,
> > -\t.intarray\t\t= {0x2, 0x3, 0x4, 0x0},
> > +\t.intarray\t\t= {0x2, 0x3, 0x4},
> >   \t.intval\t\t\t= 0x1,
> >   \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10,
> >   \t\t0x11},
> > @@ -377,7 +377,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test2 = {
> >   \t.acpi_name\t\t= "\\\\_SB.GPO0",
> >   \t.bytearray\t\t= {0x1, 0x23, 0x34},
> >   \t.byteval\t\t= 0x8,
> > -\t.intarray\t\t= {0x5, 0x0, 0x0, 0x0},
> > +\t.intarray\t\t= {0x5, 0x0, 0x0},
> >   \t.intval\t\t\t= 0x3,
> >   \t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0x0, 0x0, 0x0, 0x0,
> >   \t\t0x0},
> > diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
> > index 856392b1bd9..857861c14ed 100755
> > --- a/tools/dtoc/test_fdt.py
> > +++ b/tools/dtoc/test_fdt.py
> > @@ -379,7 +379,7 @@ class TestProp(unittest.TestCase):
> >           self.assertEqual(Type.INT, prop.type)
> >           self.assertEqual(1, fdt32_to_cpu(prop.value))
> >
> > -        # Convert singla value to array
> > +        # Convert single value to array
> >           prop2 = self.node.props['intarray']
> >           prop.Widen(prop2)
> >           self.assertEqual(Type.INT, prop.type)
> > @@ -422,6 +422,15 @@ class TestProp(unittest.TestCase):
> >           self.assertTrue(isinstance(prop.value, list))
> >           self.assertEqual(3, len(prop.value))
> >
> > +        # Widen an array of ints with an int (should do nothing)
> > +        prop = self.node.props['intarray']
> > +        prop2 = node2.props['intarray']
>
> I was expecting intval instead of intarray  here.

Yes that is a bug, thanks for spotting it. It isn't testing what it
should be. I'll come up with a fix.


>
> > +        self.assertEqual(Type.INT, prop.type)
> > +        self.assertEqual(3, len(prop.value))
> > +        prop.Widen(prop2)
> > +        self.assertEqual(Type.INT, prop.type)
> > +        self.assertEqual(3, len(prop.value))
> > +
> >       def testAdd(self):
> >           """Test adding properties"""
> >           self.fdt.pack()
>
>

Regards,
Simon