[PATCH v2 03/10] binman: Simplify comp_util class

[Simon Glass]

Hi Stefan,

On Mon, 8 Aug 2022 at 04:51, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss at weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>
> Simplify the comp_util class by remove duplicate code as preparation for
> additional compress algorithms.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
> ---
>
> (no changes since v1)
>
>  tools/binman/cbfs_util_test.py |  2 +-
>  tools/binman/comp_util.py      | 46 +++++++++++++++++++++++-----------
>  tools/binman/ftest.py          |  2 +-
>  3 files changed, 33 insertions(+), 17 deletions(-)

Reviewed-by: Simon Glass <sjg at chromium.org>

Please see below

>
> diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
> index f86b295149..44ebd04278 100755
> --- a/tools/binman/cbfs_util_test.py
> +++ b/tools/binman/cbfs_util_test.py
> @@ -50,7 +50,7 @@ class TestCbfs(unittest.TestCase):
>          cls.cbfstool = bintool.Bintool.create('cbfstool')
>          cls.have_cbfstool = cls.cbfstool.is_present()
>
> -        cls.have_lz4 = comp_util.HAVE_LZ4
> +        cls.have_lz4 = comp_util.is_present('lz4')
>
>      @classmethod
>      def tearDownClass(cls):
> diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
> index 269bbf7975..faa08a7f8a 100644
> --- a/tools/binman/comp_util.py
> +++ b/tools/binman/comp_util.py
> @@ -1,5 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0+
>  # Copyright 2022 Google LLC
> +# Copyright (C) 2022 Weidmüller Interface GmbH & Co. KG
> +# Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>  #
>  """Utilities to compress and decompress data"""
>
> @@ -8,12 +10,23 @@ import tempfile
>  from binman import bintool
>  from patman import tools
>
> -LZ4 = bintool.Bintool.create('lz4')
> -HAVE_LZ4 = LZ4.is_present()
> +# Supported compressions
> +COMPRESSIONS = ['lz4', 'lzma']

How about ALGOS ?

'Compressions' is a bit of an odd word.

>
> -LZMA_ALONE = bintool.Bintool.create('lzma_alone')
> -HAVE_LZMA_ALONE = LZMA_ALONE.is_present()
> +bintools = {}
>
> +def _get_tool_name(algo):

function comment

> +    names = {'lzma': 'lzma_alone'}
> +    return names.get(algo, algo)
> +
> +def _get_tool(algo):

function comment

> +    global bintools
> +    name = _get_tool_name(algo)
> +    tool = bintools.get(name)
> +    if not tool:
> +        tool = bintool.Bintool.create(name)
> +        bintools[name] = tool
> +    return tool
>
>  def compress(indata, algo):
>      """Compress some data using a given algorithm
> @@ -33,13 +46,12 @@ def compress(indata, algo):
>      """
>      if algo == 'none':
>          return indata
> -    if algo == 'lz4':
> -        data = LZ4.compress(indata)
> -    # cbfstool uses a very old version of lzma
> -    elif algo == 'lzma':
> -        data = LZMA_ALONE.compress(indata)
> -    else:
> +    if algo not in COMPRESSIONS:
>          raise ValueError("Unknown algorithm '%s'" % algo)
> +
> +    tool = _get_tool(algo)
> +    data = tool.compress(indata)
> +
>      return data
>
>  def decompress(indata, algo):
> @@ -60,10 +72,14 @@ def decompress(indata, algo):
>      """
>      if algo == 'none':
>          return indata
> -    if algo == 'lz4':
> -        data = LZ4.decompress(indata)
> -    elif algo == 'lzma':
> -        data = LZMA_ALONE.decompress(indata)
> -    else:
> +    if algo not in COMPRESSIONS:
>          raise ValueError("Unknown algorithm '%s'" % algo)
> +
> +    tool = _get_tool(algo)
> +    data = tool.decompress(indata)
> +
>      return data
> +
> +def is_present(algo):
> +     tool = _get_tool(algo)
> +     return tool.is_present()
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 057b4e28b7..96c15cff77 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -212,7 +212,7 @@ class TestFunctional(unittest.TestCase):
>          TestFunctional._MakeInputFile('tee.elf',
>              tools.read_file(cls.ElfTestFile('elf_sections')))
>
> -        cls.have_lz4 = comp_util.HAVE_LZ4
> +        cls.have_lz4 = comp_util.is_present('lz4')
>
>      @classmethod
>      def tearDownClass(cls):
> --
> 2.30.2
>

Regards,
Simon