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 :)