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 <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 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
signature.asc
Description: PGP signature