[U-Boot] [PATCH V4 3/3] fs: add filesystem switch libary, implement ls and fsload commands

[Andreas Bießmann]

Dear Stephen Warren,

On 22.10.2012 18:43, Stephen Warren wrote:
> From: Stephen Warren <swarren at nvidia.com>
> 
> Implement "ls" and "fsload" commands that act like {fat,ext2}{ls,load},
> and transparently handle either file-system. This scheme could easily be
> extended to other filesystem types; I only didn't do it for zfs because
> I don't have any filesystems of that type to test with.
> 
> Replace the implementation of {fat,ext[24]}{ls,load} with this new code
> too.
> 
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
> v4:
> * Use FS_TYPE_* everywhere, rather than separate FS_* and FS_TYPE_*.
> * Make fs_set_blk_dev() loop over a table of filesystems.
> v3:
> * Updated the implementation of the new commands to be suitable for use
>   as the body of {fat,ext[24]}{ls,load} too.
> * Enhanced the implementation to make more fsload parameters optional
>   (load address, filename); they can now come from environment variables
>   like ext2load supported.
> * Moved implementation into fs.c.
> * Removed cmd_ext_common.c.
> * s/partition/filesystem/ in patch subject.
> v2:
> * Build cmd_fs.c when CONFIG_CMD_FS_GENERIC not always.
> * Use new CONFIG_FS_{FAT,EXT4} rather than CONFIG_CMD_{FAT,EXT2}.
> * Implemented fs_close() and call it from the tail of fs_ls() and fs_read().
>   This ensures that any other usage of e.g. the ext4 library between fs_*()
>   calls, then the two usages won't interfere.
> * Re-organized fs.c to reduce the number of ifdef blocks.
> * Added argc checking to do_ls().
> ---
>  Makefile                |    3 +-
>  common/Makefile         |    6 +-
>  common/cmd_ext2.c       |   13 +--
>  common/cmd_ext4.c       |   12 +--
>  common/cmd_ext_common.c |  197 ------------------------------
>  common/cmd_fat.c        |   76 +-----------
>  common/cmd_fs.c         |   51 ++++++++
>  fs/Makefile             |   47 +++++++
>  fs/fs.c                 |  308 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/ext_common.h    |    3 -
>  include/fs.h            |   65 ++++++++++
>  11 files changed, 483 insertions(+), 298 deletions(-)
>  delete mode 100644 common/cmd_ext_common.c
>  create mode 100644 common/cmd_fs.c
>  create mode 100644 fs/Makefile
>  create mode 100644 fs/fs.c
>  create mode 100644 include/fs.h
> 

<snip>

> diff --git a/common/cmd_fs.c b/common/cmd_fs.c
> new file mode 100644
> index 0000000..296124b
> --- /dev/null
> +++ b/common/cmd_fs.c
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
> + *
> + * Inspired by cmd_ext_common.c, cmd_fat.c.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <fs.h>
> +
> +int do_fsload_wrapper(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	return do_fsload(cmdtp, flag, argc, argv, FS_TYPE_ANY);
> +}
> +
> +U_BOOT_CMD(
> +	fsload,	7,	0,	do_fsload_wrapper,

I just realized that cmd_jffs2.c also defines a U_BOOT_CMD fsload ...
this may also be a problem for users of the old command!

However if one defines CONFIG_CMD_JFFS2 _and_ CONFIG_CMD_FS_GENERIC the
linker will throw following warning:

---8<---
/tmp/build_avr32/common/cmd_jffs2.o:(.u_boot_list.cmd.fsload+0x0):
multiple definition of `_u_boot_list_cmd_fsload'
/tmp/build_avr32/common/cmd_fs.o:(.u_boot_list.cmd.fsload+0x0): first
defined here
/tmp/build_avr32/common/cmd_jffs2.o:(.u_boot_list.cmd.ls+0x0): multiple
definition of `_u_boot_list_cmd_ls'
/tmp/build_avr32/common/cmd_fs.o:(.u_boot_list.cmd.ls+0x0): first
defined here
--->8---

Was this intended?

Best regards

Andreas Bießmann