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 ---
tools/binman/entry.py | 6 +++--- tools/binman/entry_test.py | 8 ++++---- tools/binman/etype/_testing.py | 3 +++ tools/binman/etype/cbfs.py | 1 + tools/binman/etype/fill.py | 3 +++ tools/binman/etype/intel_ifwi.py | 1 + tools/binman/etype/section.py | 29 +++++++++++++++-------------- tools/binman/image.py | 10 +++++++--- 8 files changed, 37 insertions(+), 24 deletions(-)
diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 6436384254d..c4ddb43b318 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 460171ba899..ee729f37519 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 ae24fe8105a..4a2e4eb7caf 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 edf2189fd26..d73c706c444 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 68efe42ec0b..623b7f4e95e 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 8c79b2dd291..9cbdf3698a3 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 cd623821a34..c875a79f1ff 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 232e752258c..970d33f7110 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):