Hi Tom,

On Wed, 15 Nov 2023 at 15:38, Tom Rini <tr...@konsulko.com> wrote:
>
> On Sat, Nov 11, 2023 at 05:09:12PM -0700, Simon Glass wrote:
>
> > Rather than passing it all the command-line args, pass in the pieces
> > that it needs. These are the image address, the ramdisk address/name
> > and the FDT address/name.
> >
> > Ultimately this will allow usage of this function without being called
> > from the command line.
>
> OK, so this goal is good.
>
> [snip]
> > +             return bootm_find_images(img_addr, argc > 1 ? argv[1] : NULL,
> > +                                      argc > 2 ? argv[2] : NULL, 0, 0);
>
> That we repeat this much harder to read test/if/else three times now is
> less good.  Can we find some way to hide the complexity here in the case
> where it's coming from a command and so we have argc/argv[] ?

I can't really think of one. Ultimately this is coming from the fact
that the booti and bootz commands directly call bootm_find_images(). I
haven't got far enough to know whether that will still be true in the
end, but I hope not.

IMO the correct place for the logic above is in the command-processing
code, where it decides which arg means what.

I could imagine something like:

static const char *cmd_get_arg(int argc, char *const argv[], int argnum)
{
   return argc > argnum ? argv[argnum] : NULL;
}

but I'm not sure that is an improvement.

I have got a little further (15 more patches) and have added a 'struct
bootm_info' to hold the state of the boot, including the arguments for
the FIT address, ramdisk and fdt. But even then, the args will need to
be set up by the individual commands based on their syntax.

Regards,
Simon

Reply via email to