public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Alper Nebi Yasak <alpernebiyasak@gmail.com>
To: u-boot@lists.denx.de
Subject: [PATCH 10/25] binman: Move section-building code into a function
Date: Tue, 27 Oct 2020 02:17:25 +0300	[thread overview]
Message-ID: <ba75cd61-b2dd-24ee-e575-aa9b16a01d90@gmail.com> (raw)
In-Reply-To: <CAPnjgZ2KYfrNEu_LqwMvxk-Bd8oahPQN-BUiSfdUEJzrsKVERA@mail.gmail.com>

On 26/10/2020 22:22, Simon Glass wrote:
> On Mon, 19 Oct 2020 at 15:29, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>> On 19/10/2020 05:41, Simon Glass wrote:
>>>          for entry in self._entries.values():
>>>              data = entry.GetData()
>>> -            base = self.pad_before + (entry.offset or 0) - self._skip_at_start
>>> -            pad = base - len(section_data) + (entry.pad_before or 0)
>>> +            # Handle empty space before the entry
>>> +            pad = (entry.offset or 0) - self._skip_at_start - len(section_data)
>>>              if pad > 0:
>>>                  section_data += tools.GetBytes(self._pad_byte, pad)
>>> +
>>> +            # Handle padding before the entry
>>> +            if entry.pad_before:
>>> +                section_data += tools.GetBytes(self._pad_byte, entry.pad_before)
>>
>> Consider this fragment:
>>
>>     section {
>>         skip-at-start = <16>;
>>
>>         blob {
>>             pad-before = <16>;
>>             filename = "foo";
>>         }
>>     }
>>
>> Is this invalid as 'blob' isn't offset > skip-at-start? This wouldn't
>> be padded in the earlier code, but would be padded after this (assuming
>> it doesn't error out) -- was that a bug or undefined behaviour or
>> something?
> 
> You have very sharp eyes.

Thanks! I had actually tried to copy this to the fit etype before making
it use the section etype directly, so I did read and think about this
part (and Pack()) a lot to better understand how things were supposed to
work.

> The case you list seems to produce the same result before and after
> this patch. But if I put the padding at the top level it does strange
> things, so perhaps that's what you mean?

I was trying to write a case where a pad = (-16) + (16) = 0 gets split
into two (pad = -16, then entry.pad_before = 16) after that change,
causing a change in padding. Still looks correct to me, but I didn't
actually run anything so I'm probably getting something wrong.

> I've added a few test cases along these lines in v2, and one of them
> certainly different behaviour. This is good actually since it shows a
> simple case of what these padding changes are intended to fix.

  reply	other threads:[~2020-10-26 23:17 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-19  2:41 [PATCH 00/25] binman: Support compression of sections Simon Glass
2020-10-19  2:41 ` [PATCH 01/25] binman: Give a sensible error if no command is given Simon Glass
2020-10-19  2:41 ` [PATCH 02/25] binman: Fix return from u-boot-ucode if there is no DT Simon Glass
2020-10-19  2:41 ` [PATCH 03/25] binman: Remove references to 'image' in entry_Section Simon Glass
2020-10-19  2:41 ` [PATCH 04/25] binman: Expand the error message for breaching a section Simon Glass
2020-10-19  2:41 ` [PATCH 05/25] binman: Move CompressData() into Entry base class Simon Glass
2020-10-19  2:41 ` [PATCH 06/25] binman: Use 'files-compress' to set compression for files Simon Glass
2020-10-19 19:01   ` Alper Nebi Yasak
2020-10-19  2:41 ` [PATCH 07/25] binman: Update testPackExtra with more checks Simon Glass
2020-10-19  2:41 ` [PATCH 08/25] binman: Expand docs and test for padding Simon Glass
2020-10-19  2:41 ` [PATCH 09/25] binman: Expand docs and test for alignment Simon Glass
2020-10-19  2:41 ` [PATCH 10/25] binman: Move section-building code into a function Simon Glass
2020-10-19 20:30   ` Alper Nebi Yasak
2020-10-26 19:22     ` Simon Glass
2020-10-26 23:17       ` Alper Nebi Yasak [this message]
2020-11-03 15:11         ` Simon Glass
2020-11-04 21:50           ` Alper Nebi Yasak
2021-01-23 16:15             ` Simon Glass
2020-10-19  2:41 ` [PATCH 11/25] binman: Refactor _BuildSectionData() Simon Glass
2020-10-19  2:41 ` [PATCH 12/25] binman: Move section padding to the parent Simon Glass
2020-10-19  2:41 ` [PATCH 13/25] binman: Make section padding consistent with other entries Simon Glass
2020-10-19  2:41 ` [PATCH 14/25] binman: Store the original data before compression Simon Glass
2020-10-19  2:41 ` [PATCH 15/25] binman: Set section contents in GetData() Simon Glass
2020-10-19  2:41 ` [PATCH 16/25] binman: Avoid reporting image-pos with compression Simon Glass
2020-10-19 21:15   ` Alper Nebi Yasak
2020-10-26 19:22     ` Simon Glass
2020-10-26 22:25       ` Alper Nebi Yasak
2020-10-30 18:15         ` Simon Glass
2020-11-04 19:45           ` Alper Nebi Yasak
2020-10-19  2:41 ` [PATCH 17/25] binman: Drop Entry.CheckOffset() Simon Glass
2020-10-19  2:41 ` [PATCH 18/25] binman: Move sort and expand to the main Pack() function Simon Glass
2020-10-19  2:41 ` [PATCH 19/25] binman: Drop the Entry.CheckSize() method Simon Glass
2020-10-19  2:41 ` [PATCH 20/25] binman: Call CheckSize() from the section's Pack() method Simon Glass
2020-10-19  2:41 ` [PATCH 21/25] binman: Drop CheckEntries() Simon Glass
2020-10-19  2:41 ` [PATCH 22/25] binman: Update CheckEntries() for compressed sections Simon Glass
2020-10-19  2:41 ` [PATCH 23/25] binman: Use the actual contents in CheckSize() Simon Glass
2020-10-19  2:41 ` [PATCH 24/25] binman: Support compression of sections Simon Glass
2020-10-19  2:41 ` [PATCH 25/25] binman: Avoid calculated section data repeatedly Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ba75cd61-b2dd-24ee-e575-aa9b16a01d90@gmail.com \
    --to=alpernebiyasak@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox