On Mon, 2015-01-05 at 13:18 -0700, Stephen Warren wrote: > On 01/05/2015 10:13 AM, Sjoerd Simons wrote: > > New command to determine the filesystem type of a given partition. > > Optionally stores the filesystem type in a environment variable. > > > diff --git a/common/cmd_fs.c b/common/cmd_fs.c > > > +U_BOOT_CMD( > > + fstype, 4, 1, do_fstype_wrapper, > > + "Look up a filesystem type", > > + "<interface> <dev>:<part>\n" > > Should this line ... > > > + "- print filesystem type\n" > > + "fstype <interface> <dev>:<part> <varname>\n" > > ... be consistent with this one - namely either both or neither include > "fstype" at the start?
Nope, the cmd_usage implementation does (summarized):
printf("Usage:\n%s ", cmdtp->name);
puts(cmdtp->help);
putc('\n');
So the "fstype" at the start of the first line gets added by that code,
hence the declaration needs to be inconsistent to have a consistent
output for the user :)
> > diff --git a/fs/fs.c b/fs/fs.c
>
> > +int do_fs_type(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> > +{
> > + struct fstype_info *info;
> > +
> > + if (argc < 3 || argc > 4)
> > + return CMD_RET_USAGE;
> > +
> > + if (fs_set_blk_dev(argv[1], argv[2], FS_TYPE_ANY))
> > + return 1;
> > +
> > + info = fs_get_info(fs_type);
> > +
> > + if (argc == 4)
> > + setenv(argv[3], info->name);
> > + else
> > + printf("%s\n", info->name);
> > +
> > + return CMD_RET_SUCCESS;
> > +}
>
> That function has both the cmdline interface and implementation logic in
> one place. Many of the other features (read, write, ls, ...) separate
> them out into two functions so that U-Boot code can call the
> implementation, and shell code can call the cmdline parsing wrapper.
> Should we do the same here? Admittedly, the implementation would be
> pretty simple, but perhaps it's still useful?
Ah i did wonder why the splitup was there in some functions, that
explains it :).
I would expect that u-boot code which would like to use it would be more
interested in getting the fstype struct rather then necessarily the name
as a string (or printed out).. But i could well be wrong.
> I don't feel that strongly though, and we can easily refactor that later
> if required.
Same here, although i'd slightly prefer refactoring if-needed rather
then pre-emptively :)
--
Sjoerd Simons <sjoerd.sim...@collabora.co.uk>
Collabora Ltd.
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
