[PATCH 1/3] dtoc: Rename is_wider_than() to reduce confusion

Walter Lozano wlozano at collabora.com
Mon Aug 2 04:45:41 CEST 2021


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)

>   
> -    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,

Walter



More information about the U-Boot mailing list