binman: Adjust Entry to read the node in a separate call

At present the Entry constructor sets up the object and then immediately
reads its device-tree node to obtain its properties.

This breaks a convention that constructors should not do any processing.
A consequence is that we must pass all arguments to the constructor and
cannot have the node-reading proceed in a different way unless we pass
flags to that constructor. We already have a 'test' flag in a few cases,
and now need to control whether the 'orig_offset' and 'orig_size'
properties are set or not.

Adjust the code to require a separate call to ReadNode() after
construction. The Image class remains as it was.

Signed-off-by: Simon Glass <sjg@chromium.org>

8 files changed, 37 insertions, 24 deletions

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 6436384254..c4ddb43b31 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -70,7 +70,7 @@ class Entry(object):
         orig_offset: Original offset value read from node
         orig_size: Original size value read from node
     """
-    def __init__(self, section, etype, node, read_node=True, name_prefix=''):
+    def __init__(self, section, etype, node, name_prefix=''):
         self.section = section
         self.etype = etype
         self._node = node
@@ -89,8 +89,6 @@ class Entry(object):
         self.image_pos = None
         self._expand_size = False
         self.compress = 'none'
-        if read_node:
-            self.ReadNode()
 
     @staticmethod
     def Lookup(node_path, etype):
@@ -155,6 +153,8 @@ class Entry(object):
     def ReadNode(self):
         """Read entry information from the node
 
+        This must be called as the first thing after the Entry is created.
+
         This reads all the fields we recognise from the node, ready for use.
         """
         if 'pos' in self._node.props:
diff --git a/tools/binman/entry_test.py b/tools/binman/entry_test.py
index 460171ba89..ee729f3751 100644
--- a/tools/binman/entry_test.py
+++ b/tools/binman/entry_test.py
@@ -57,7 +57,7 @@ class TestEntry(unittest.TestCase):
     def testEntryContents(self):
         """Test the Entry bass class"""
         import entry
-        base_entry = entry.Entry(None, None, None, read_node=False)
+        base_entry = entry.Entry(None, None, None)
         self.assertEqual(True, base_entry.ObtainContents())
 
     def testUnknownEntry(self):
@@ -73,15 +73,15 @@ class TestEntry(unittest.TestCase):
         """Test Entry.GetUniqueName"""
         Node = collections.namedtuple('Node', ['name', 'parent'])
         base_node = Node('root', None)
-        base_entry = entry.Entry(None, None, base_node, read_node=False)
+        base_entry = entry.Entry(None, None, base_node)
         self.assertEqual('root', base_entry.GetUniqueName())
         sub_node = Node('subnode', base_node)
-        sub_entry = entry.Entry(None, None, sub_node, read_node=False)
+        sub_entry = entry.Entry(None, None, sub_node)
         self.assertEqual('root.subnode', sub_entry.GetUniqueName())
 
     def testGetDefaultFilename(self):
         """Trivial test for this base class function"""
-        base_entry = entry.Entry(None, None, None, read_node=False)
+        base_entry = entry.Entry(None, None, None)
         self.assertIsNone(base_entry.GetDefaultFilename())
 
     def testBlobFdt(self):
diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index ae24fe8105..4a2e4eb7ca 100644
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -42,6 +42,9 @@ class Entry__testing(Entry):
     """
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
+
+    def ReadNode(self):
+        Entry.ReadNode(self)
         self.return_invalid_entry = fdt_util.GetBool(self._node,
                                                      'return-invalid-entry')
         self.return_unknown_contents = fdt_util.GetBool(self._node,
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index edf2189fd2..d73c706c44 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -203,6 +203,7 @@ class Entry_cbfs(Entry):
         """Read the subnodes to find out what should go in this IFWI"""
         for node in self._node.subnodes:
             entry = Entry.Create(self.section, node)
+            entry.ReadNode()
             entry._cbfs_name = fdt_util.GetString(node, 'cbfs-name', entry.name)
             entry._type = fdt_util.GetString(node, 'cbfs-type')
             compress = fdt_util.GetString(node, 'cbfs-compress', 'none')
diff --git a/tools/binman/etype/fill.py b/tools/binman/etype/fill.py
index 68efe42ec0..623b7f4e95 100644
--- a/tools/binman/etype/fill.py
+++ b/tools/binman/etype/fill.py
@@ -23,6 +23,9 @@ class Entry_fill(Entry):
     """
     def __init__(self, section, etype, node):
         Entry.__init__(self, section, etype, node)
+
+    def ReadNode(self):
+        Entry.ReadNode(self)
         if self.size is None:
             self.Raise("'fill' entry must have a size property")
         self.fill_value = fdt_util.GetByte(self._node, 'fill-byte', 0)
diff --git a/tools/binman/etype/intel_ifwi.py b/tools/binman/etype/intel_ifwi.py
index 8c79b2dd29..9cbdf3698a 100644
--- a/tools/binman/etype/intel_ifwi.py
+++ b/tools/binman/etype/intel_ifwi.py
@@ -94,6 +94,7 @@ class Entry_intel_ifwi(Entry_blob):
         """Read the subnodes to find out what should go in this IFWI"""
         for node in self._node.subnodes:
             entry = Entry.Create(self.section, node)
+            entry.ReadNode()
             entry._ifwi_replace = fdt_util.GetBool(node, 'replace')
             entry._ifwi_subpart = fdt_util.GetString(node, 'ifwi-subpart')
             entry._ifwi_entry_name = fdt_util.GetString(node, 'ifwi-entry')
diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index cd623821a3..c875a79f1f 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -52,22 +52,10 @@ class Entry_section(Entry):
         self._sort = False
         self._skip_at_start = None
         self._end_4gb = False
-        if not test:
-            self._ReadNode()
-            self._ReadEntries()
-
-    def _Raise(self, msg):
-        """Raises an error for this section
-
-        Args:
-            msg: Error message to use in the raise string
-        Raises:
-            ValueError()
-        """
-        raise ValueError("Section '%s': %s" % (self._node.path, msg))
 
-    def _ReadNode(self):
+    def ReadNode(self):
         """Read properties from the image node"""
+        Entry.ReadNode(self)
         self._pad_byte = fdt_util.GetInt(self._node, 'pad-byte', 0)
         self._sort = fdt_util.GetBool(self._node, 'sort-by-offset')
         self._end_4gb = fdt_util.GetBool(self._node, 'end-at-4gb')
@@ -87,14 +75,27 @@ class Entry_section(Entry):
         if filename:
             self._filename = filename
 
+        self._ReadEntries()
+
     def _ReadEntries(self):
         for node in self._node.subnodes:
             if node.name == 'hash':
                 continue
             entry = Entry.Create(self, node)
+            entry.ReadNode()
             entry.SetPrefix(self._name_prefix)
             self._entries[node.name] = entry
 
+    def _Raise(self, msg):
+        """Raises an error for this section
+
+        Args:
+            msg: Error message to use in the raise string
+        Raises:
+            ValueError()
+        """
+        raise ValueError("Section '%s': %s" % (self._node.path, msg))
+
     def GetFdts(self):
         fdts = {}
         for entry in self._entries.values():
diff --git a/tools/binman/image.py b/tools/binman/image.py
index 232e752258..970d33f711 100644
--- a/tools/binman/image.py
+++ b/tools/binman/image.py
@@ -50,9 +50,13 @@ class Image(section.Entry_section):
         self.fdtmap_dtb = None
         self.fdtmap_data = None
         if not test:
-            filename = fdt_util.GetString(self._node, 'filename')
-            if filename:
-                self._filename = filename
+            self.ReadNode()
+
+    def ReadNode(self):
+        section.Entry_section.ReadNode(self)
+        filename = fdt_util.GetString(self._node, 'filename')
+        if filename:
+            self._filename = filename
 
     @classmethod
     def FromFile(cls, fname):