Re: [PATCH v2 08/25] binman: Rename ExpandToLimit to extend_to_limit

Alper Nebi Yasak
2022-03-03T21:16:57Z

On 24/02/2022 02:00, Simon Glass wrote:
> The word 'expand' is used for entries which generate subentries. It is
> also used for entries that can have an '_expanded' version which is used
> to break out its contents.
>
> Rather than talking about expanding an entry's size, use the term
> 'extending'. It is slightly more precise and avoids the above conflicts.
>
> This change renders the old 'expand-size' property invalid, so add an
> error check for that.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Add patch to rename ExpandToLimit to extend_to_limit
>
> tools/binman/binman.rst | 2 +-
> tools/binman/entry.py | 8 +++++---
> tools/binman/etype/section.py | 10 +++++-----
> tools/binman/ftest.py | 8 ++++++++
> tools/binman/test/088_expand_size.dts | 8 ++++----
> tools/binman/test/089_expand_size_bad.dts | 2 +-
> tools/binman/test/225_expand_size_bad.dts | 10 ++++++++++
> 7 files changed, 34 insertions(+), 14 deletions(-)
> create mode 100644 tools/binman/test/225_expand_size_bad.dts

I haven't ever used expand-size, but:

Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

> diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
> index 0061e43659..85f8337a31 100644
> --- a/tools/binman/binman.rst
> +++ b/tools/binman/binman.rst
> @@ -480,7 +480,7 @@ image-pos:
> for each entry. This makes it easy to find out exactly where the entry
> ended up in the image, regardless of parent sections, etc.
>
> -expand-size:
> +extend-size:
> Expand the size of this entry to fit available space. This space is only

Expand -> Extend

> limited by the size of the image/section and the position of the next
> entry.
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index de8526618d..a59eb56f14 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -233,6 +233,8 @@ class Entry(object):
> """
> if 'pos' in self._node.props:
> self.Raise("Please use 'offset' instead of 'pos'")
> + if 'expand-size' in self._node.props:
> + self.Raise("Please use 'extend-size' instead of 'expand-size'")
> self.offset = fdt_util.GetInt(self._node, 'offset')
> self.size = fdt_util.GetInt(self._node, 'size')
> self.orig_offset = fdt_util.GetInt(self._node, 'orig-offset')
> @@ -260,7 +262,7 @@ class Entry(object):
> self.align_size)
> self.align_end = fdt_util.GetInt(self._node, 'align-end')
> self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset')
> - self.expand_size = fdt_util.GetBool(self._node, 'expand-size')
> + self.expand_size = fdt_util.GetBool(self._node, 'extend-size')

Consider also changing the attribute name, I don't see many instances of
it with git-grep. Maybe even the test dts names.

> self.missing_msg = fdt_util.GetString(self._node, 'missing-msg')
>
> # This is only supported by blobs and sections at present
> @@ -772,8 +774,8 @@ features to produce new behaviours.
> name = '%s.%s' % (node.name, name)
> return name
>
> - def ExpandToLimit(self, limit):
> - """Expand an entry so that it ends at the given offset limit"""
> + def extend_to_limit(self, limit):
> + """Extent an entry so that it ends at the given offset limit"""

Extent -> Extend

> if self.offset + self.size < limit:
> self.size = limit - self.offset
> # Request the contents again, since changing the size requires that
> …