[PATCH 27/29] bootm: Adjust the parameters of bootm_find_images()

[Tom Rini]

On Wed, Nov 15, 2023 at 06:42:19PM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Wed, 15 Nov 2023 at 15:38, Tom Rini <trini at 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 was thinking about this more after I sent this. And I think we might
indeed want an inline / macro to handle this case more generally as I
suspect we'll have similar cases where we need argN-or-NULL as we
refactor other areas of code to split "here is the command" from "here
is the library functionality" of it. Then we'll have:
ulong foo = cmd_arg_one(argc, argv);
ulong bar = cmd_arg_two(argc, argv);

librarycall(foo, bar, ...);

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231115/976e8b7e/attachment.sig>