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 <s...@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

Reply via email to