On 10/30/2012 03:59 PM, Benoît Thébaudeau wrote: > On Tuesday, October 30, 2012 10:18:03 PM, Stephen Warren wrote: >> On 10/30/2012 02:23 PM, Benoît Thébaudeau wrote: >>>> +int do_ls(cmd_tbl_t *cmdtp, int flag, int argc, char * const >>>> argv[], >>>> + int fstype) >>>> +{ >>>> + if (argc < 2) >>>> + return CMD_RET_USAGE; >>>> + >>>> + if (fs_set_blk_dev(argv[1], (argc >= 3) ? argv[2] : NULL, >>>> fstype)) >>>> + return 1; >>>> + >>>> + if (fs_ls(argc == 4 ? argv[3] : "/")) >>> >>> IMHO, it would be better to just ignore the possible extra >>> arguments, like in: >>> + if (fs_ls(argc >= 4 ? argv[3] : "/")) >> >> Here I don't agree. If the command expects a certain set of >> arguments, >> we should validate that the user provided exactly that set, and no >> more. >> If we allow arbitrary cruft, then if we need to add new arguments >> later, >> we won't be able to guarantee that handling those extra arguments >> won't >> break some existing broken usage of the command. > > My comment was misleading. Actually, with the current code, do_ls() can not be > called (except directly) if there are more than 4 arguments, because of the > way > the ls command is declared through U_BOOT_CMD(). Hence, if ">=" is used, > arguments can be added later without changing existing lines.
Ah OK, that makes sense. > And if we consider a direct call to do_ls() skipping the command system, then > this function should return CMD_RET_USAGE if argc > 4. True. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot