On Sun, 27 Mar 2022 at 09:32, Alper Nebi Yasak <alpernebiya...@gmail.com> wrote:
>
> Binman interfaces allow attempts to replace any entry in the image with
> arbitrary data. When trying to replace sections, the changes in the
> section entry's data are not propagated to its child entries. This,
> combined with how sections rebuild their contents from its children,
> eventually causes the replaced contents to be silently overwritten by
> rebuilt contents equivalent to the original data.
>
> Add a simple test for replacing a section that is currently failing due
> to this behaviour, and mark it as an expected failure. Also, raise an
> error when replacing a section instead of silently pretending it was
> replaced.
>
> Signed-off-by: Alper Nebi Yasak <alpernebiya...@gmail.com>
> ---
>
> tools/binman/etype/section.py | 3 +++
> tools/binman/ftest.py | 9 ++++++++
> .../test/234_replace_section_simple.dts | 23 +++++++++++++++++++
> 3 files changed, 35 insertions(+)
> create mode 100644 tools/binman/test/234_replace_section_simple.dts
Reviewed-by: Simon Glass <s...@chromium.org>
Is it not better to assertRaises() and check that the error message is
as expected?
>
> diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
> index ccac658c1831..bd67238b9199 100644
> --- a/tools/binman/etype/section.py
> +++ b/tools/binman/etype/section.py
> @@ -788,6 +788,9 @@ def ReadChildData(self, child, decomp=True,
> alt_format=None):
> data = new_data
> return data
>
> + def WriteData(self, data, decomp=True):
> + self.Raise("Replacing sections is not implemented yet")
> +
> def WriteChildData(self, child):
> return True
>
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 43bec4a88841..058651cc18a0 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -5641,6 +5641,15 @@ def testReplaceFitSubentryLeafSmallerSize(self):
> self.assertIsNotNone(path)
> self.assertEqual(expected_fdtmap, fdtmap)
>
> + @unittest.expectedFailure
> + def testReplaceSectionSimple(self):
> + """Test replacing a simple section with arbitrary data"""
> + new_data = b'w' * len(COMPRESS_DATA + U_BOOT_DATA)
> + data, expected_fdtmap, _ = self._RunReplaceCmd(
> + 'section', new_data,
> + dts='234_replace_section_simple.dts')
> + self.assertEqual(new_data, data)
> +
>
> if __name__ == "__main__":
> unittest.main()
> diff --git a/tools/binman/test/234_replace_section_simple.dts
> b/tools/binman/test/234_replace_section_simple.dts
> new file mode 100644
> index 000000000000..c9d5c3285615
> --- /dev/null
> +++ b/tools/binman/test/234_replace_section_simple.dts
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> + binman {
> + allow-repack;
> +
> + u-boot-dtb {
> + };
> +
> + section {
> + blob {
> + filename = "compress";
> + };
> +
> + u-boot {
> + };
> + };
> +
> + fdtmap {
> + };
> + };
> +};
> --
> 2.35.1
>
