[2/3] binman: Generate the preload header and sign the data only once

[Paul HENRYS]

Hi Simon,

On 02/04/2026 02:17, Simon Glass wrote:
> This Mail comes from Outside of SoftAtHome: Do not answer, click links or open attachments unless you recognize the sender and know the content is safe.
>
> Hi Paul,
>
> On 2026-04-01T15:35:23, yan wang <yan.wang at softathome.com> wrote:
>> binman: Generate the preload header and sign the data only once
>> binman: Generate the preload header and sign the data only once
>>
>> To optimize the preload generation, the header and signatures are
>> only generated once all data have been collected. This avoids to
>> sign multiple times.
>>
>> Signed-off-by: Paul HENRYS <paul.henrys_ext at softathome.com>
>> diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
>> @@ -152,14 +152,12 @@ class Entry_pre_load(Entry_collection):
>> +    def ObtainContents(self):
>> +        """Do nothing here.
>> +        The data will be obtained and handled in ProcessContents
>> +        """
>> +        return True
> Since ObtainContents() no longer calls SetContents(), contents_size
> remains 0 during Pack(). This forces an extra packing pass when
> ProcessContentsUpdate() detects the size change from 0 to header_size.
>
> Since header_size is known upfront (from __init__), you could set
> contents_size here to avoid the re-pack:
>
>      def ObtainContents(self):
>          self.contents_size = self.header_size
>          return True
>
> This would allow Pack() to reserve the correct space immediately.
Good idea. Thx.
>
>> diff --git a/tools/binman/etype/pre_load.py b/tools/binman/etype/pre_load.py
>> @@ -152,14 +152,12 @@ class Entry_pre_load(Entry_collection):
>> +    def ProcessContents(self):
>> +        self.image = self.GetContents(True)
>> +        data = self._CreateHeader()
>> +        return self.ProcessContentsUpdate(data)
> Looking at the series as a whole, patch 3/3 adds the build_done
> marking in GetContents() which prevents referenced entries from being
> rebuilt. However, this patch calls GetContents() unconditionally with
> required=True. If the referenced entry's data somehow becomes
> unavailable between passes, this would raise an exception rather than
> returning gracefully.
>
> I suspect this is fine since ProcessContents() runs after
> ObtainContents() has succeeded for all entries, but please confirm
> this is the intended behaviour.
Actually, the default implementation of `ProcessContents()` in the 
`collection` class calls `GetContents(required=True)`.
The patch restores this call before header generation because the data 
are no longer retrieved and the header is no longer generated in 
`ObtainContents()`.

In addition, the `GetContents()` implementation in the `collection` 
class would not raise an exception if the data were `None`; instead, no 
data would be added for that entry, if my understanding is correct.

This series of patches aims to generate the preload header only once in 
`ProcessContents()` as an optimization and to prevent the data from 
changing after signing the entry contents with the preload header, the 
`build_done` property is set to `True` for every entry referenced by the 
preload.
>
> Regards,
> Simon

We will push a v2 of the initial patches taking into account the remarks 
you made. Thx.

Regards,
Paul