On Tue, 9 Sep 2025, Orzel, Michal wrote:
> On 09/09/2025 11:22, Mykola Kvach wrote:
> > Hi Michal,
> >
> > Thank you for the patch and the detailed explanation.
> >
> > On Tue, Sep 9, 2025 at 10:42 AM Michal Orzel <michal.or...@amd.com> wrote:
> >>
> >> Commit 061d6782756f modified load_file() to take load command as
> >> argument but did not change all the invocations (e.g. loading standalone
> >> Linux, bitstream, etc.) which broke the output script (load command
> >> empty). Fix it by defaulting to LOAD_CMD if not specified.
> >>
> >> Fixes: 061d6782756f ("Add config option to use separate load commands for
> >> Xen, DOM0 and DOMU binaries")
> >> Signed-off-by: Michal Orzel <michal.or...@amd.com>
> >> ---
> >> scripts/uboot-script-gen | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> >> index 849b8f939e81..4f9261035d73 100755
> >> --- a/scripts/uboot-script-gen
> >> +++ b/scripts/uboot-script-gen
> >> @@ -736,6 +736,12 @@ function load_file()
> >> local base="$(realpath $PWD)"/
> >> local relative_path=${absolute_path#"$base"}
> >>
> >> + # Default to LOAD_CMD if not specified
> >> + if test -z "${load_cmd}"
> >> + then
> >> + load_cmd="${LOAD_CMD}"
> >> + fi
> >> +
> >
> > I was wondering if we could use a slightly more concise notation here, like:
> > : "${load_cmd:=$LOAD_CMD}"
> >
> > It does the same thing but is a bit more idiomatic for Bash scripts.
> Some time ago, Stefano requested me to use a simpler notation in ImageBuilder,
> so that it's immediately clear what the script does. Therefore I followed this
> suggestion here as well. I will let him choose what suits the project best.
I prefer the way Michal wrote it.
This is one of those cases where there is no right or wrong answer.
Mykola's suggestion is a more modern and concise version. The code style
I used was an attempt to be more verbose but also clearer. As always,
when it comes to clarity, each individual finds different approaches
more understandable.
