[PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion
Simon Glass
sjg at chromium.org
Mon Aug 2 04:50:26 CEST 2021
Hi Walter,
On Sun, 1 Aug 2021 at 20:45, Walter Lozano <wlozano at collabora.com> wrote:
>
> Hi Simon,
>
> Thanks for checking this bug, I'm glad that you were able to come with
> fix quickly. I have some questions, I hope that you find some time to
> help me understand.
>
> On 7/28/21 10:23 PM, Simon Glass wrote:
> > The current name is confusing because the logic is actually backwards from
> > what you might expect. Rename it to needs_widening() and update the
> > comments.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > tools/dtoc/fdt.py | 15 +++++++++------
> > 1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
> > index 3996971e39c..9749966d5fb 100644
> > --- a/tools/dtoc/fdt.py
> > +++ b/tools/dtoc/fdt.py
> > @@ -24,16 +24,19 @@ from patman import tools
> >
> > # A list of types we support
> > class Type(IntEnum):
> > + # Types in order from widest to narrowest
> > (BYTE, INT, STRING, BOOL, INT64) = range(5)
>
> Sorry but I don't understand why BYTE is wider than INT (or INT64)
I think perhaps we need a better name. A wider type is one that can
hold the values of a narrower one, plus more.
In this case a 'bytes' type can hold anything (bytes, int, int64,
bool) so is the 'widest' there is. It is the lowest common denominator
in the devicetree.
>
> >
> > - def is_wider_than(self, other):
> > - """Check if another type is 'wider' than this one
> > + def needs_widening(self, other):
> > + """Check if this type needs widening to hold a value from another type
> >
> > - A wider type is one that holds more information than an earlier one,
> > - similar to the concept of type-widening in C.
> > + A wider type is one that can hold a wider array of information than
> > + another one, or is less restrictive, so it can hold the information of
> > + another type as well as its own. This is similar to the concept of
> > + type-widening in C.
> >
> > This uses a simple arithmetic comparison, since type values are in order
> > - from narrowest (BYTE) to widest (INT64).
> > + from widest (BYTE) to narrowest (INT64).
> >
> > Args:
> > other: Other type to compare against
> > @@ -149,7 +152,7 @@ class Prop:
> > update the current property to be like the second, since it is less
> > specific.
> > """
> > - if self.type.is_wider_than(newprop.type):
> > + if self.type.needs_widening(newprop.type):
> > if self.type == Type.INT and newprop.type == Type.BYTE:
> > if type(self.value) == list:
> > new_value = []
>
Regards,
Simon
More information about the U-Boot
mailing list