[U-Boot] [PATCH 1/4] fs: Add command to retrieve the filesystem type

Sjoerd Simons
2015-01-06T16:40:23Z

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.simons@collabora.co.uk>
Collabora Ltd.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 6170 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150106/58c2687c/attachment.bin>