[U-Boot] [PATCH v2 1/4] ubifs: Modify ubifs u-boot wrapper function prototypes for generic fs use

[Heiko Schocher <hs@denx.de>]

Hello Hans,

Am 22.08.2015 um 20:04 schrieb Hans de Goede:
> Modify the ubifs u-boot wrapper function prototypes for generic fs use,
> and give them their own header file.
>
> This is a preparation patch for adding ubifs support to the generic fs
> code from fs/fs.c.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>   common/cmd_ubifs.c    | 14 +++--------
>   fs/ubifs/ubifs.c      | 70 ++++++++++++++++++++++++++++++++++++++++-----------
>   fs/ubifs/ubifs.h      |  6 +----
>   include/ubifs_uboot.h | 29 +++++++++++++++++++++
>   4 files changed, 89 insertions(+), 30 deletions(-)
>   create mode 100644 include/ubifs_uboot.h

Only one nitpick, beside of this, you can add my:

Reviewed-by: Heiko Schocher <hs@denx.de>

> diff --git a/common/cmd_ubifs.c b/common/cmd_ubifs.c
> index 8e9a4e5..0a3dd24 100644
> --- a/common/cmd_ubifs.c
> +++ b/common/cmd_ubifs.c
> @@ -15,11 +15,10 @@
>   #include <common.h>
>   #include <config.h>
>   #include <command.h>
> -
> -#include "../fs/ubifs/ubifs.h"
> +#include <ubifs_uboot.h>
>
>   static int ubifs_initialized;
> -static int ubifs_mounted;
> +int ubifs_mounted;

later you add "extern int ubifs_mounted" in include/ubifs_uboot.h

Maybe you want to add a function, which returns the state
of this var and let the var static?

bye,
Heiko
>   static int do_ubifs_mount(cmd_tbl_t *cmdtp, int flag, int argc,
>   				char * const argv[])
> @@ -54,14 +53,7 @@ int ubifs_is_mounted(void)
>
>   void cmd_ubifs_umount(void)
>   {
> -
> -	if (ubifs_sb) {
> -		printf("Unmounting UBIFS volume %s!\n",
> -		       ((struct ubifs_info *)(ubifs_sb->s_fs_info))->vi.name);
> -		ubifs_umount(ubifs_sb->s_fs_info);
> -	}
> -
> -	ubifs_sb = NULL;
> +	uboot_ubifs_umount();
>   	ubifs_mounted = 0;
>   	ubifs_initialized = 0;
>   }
> diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
> index 6dd6174..5af861c 100644
> --- a/fs/ubifs/ubifs.c
> +++ b/fs/ubifs/ubifs.c
> @@ -568,7 +568,7 @@ static unsigned long ubifs_findfile(struct super_block *sb, char *filename)
>   	return 0;
>   }
>
> -int ubifs_ls(char *filename)
> +int ubifs_ls(const char *filename)
>   {
>   	struct ubifs_info *c = ubifs_sb->s_fs_info;
>   	struct file *file;
> @@ -579,7 +579,7 @@ int ubifs_ls(char *filename)
>   	int ret = 0;
>
>   	c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY);
> -	inum = ubifs_findfile(ubifs_sb, filename);
> +	inum = ubifs_findfile(ubifs_sb, (char *)filename);
>   	if (!inum) {
>   		ret = -1;
>   		goto out;
> @@ -785,7 +785,8 @@ error:
>   	return err;
>   }
>
> -int ubifs_load(char *filename, u32 addr, u32 size)
> +int ubifs_read(const char *filename, void *buf, loff_t offset,
> +	       loff_t size, loff_t *actread)
>   {
>   	struct ubifs_info *c = ubifs_sb->s_fs_info;
>   	unsigned long inum;
> @@ -796,10 +797,18 @@ int ubifs_load(char *filename, u32 addr, u32 size)
>   	int count;
>   	int last_block_size = 0;
>
> +	*actread = 0;
> +
> +	if (offset & (PAGE_SIZE - 1)) {
> +		printf("ubifs: Error offset must be a multple of %d\n",
> +		       PAGE_SIZE);
> +		return -1;
> +	}
> +
>   	c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY);
>   	/* ubifs_findfile will resolve symlinks, so we know that we get
>   	 * the real file here */
> -	inum = ubifs_findfile(ubifs_sb, filename);
> +	inum = ubifs_findfile(ubifs_sb, (char *)filename);
>   	if (!inum) {
>   		err = -1;
>   		goto out;
> @@ -815,19 +824,24 @@ int ubifs_load(char *filename, u32 addr, u32 size)
>   		goto out;
>   	}
>
> +	if (offset > inode->i_size) {
> +		printf("ubifs: Error offset (%lld) > file-size (%lld)\n",
> +		       offset, size);
> +		err = -1;
> +		goto put_inode;
> +	}
> +
>   	/*
>   	 * If no size was specified or if size bigger than filesize
>   	 * set size to filesize
>   	 */
> -	if ((size == 0) || (size > inode->i_size))
> -		size = inode->i_size;
> +	if ((size == 0) || (size > (inode->i_size - offset)))
> +		size = inode->i_size - offset;
>
>   	count = (size + UBIFS_BLOCK_SIZE - 1) >> UBIFS_BLOCK_SHIFT;
> -	printf("Loading file '%s' to addr 0x%08x with size %d (0x%08x)...\n",
> -	       filename, addr, size, size);
>
> -	page.addr = (void *)addr;
> -	page.index = 0;
> +	page.addr = buf;
> +	page.index = offset / PAGE_SIZE;
>   	page.inode = inode;
>   	for (i = 0; i < count; i++) {
>   		/*
> @@ -844,16 +858,44 @@ int ubifs_load(char *filename, u32 addr, u32 size)
>   		page.index++;
>   	}
>
> -	if (err)
> +	if (err) {
>   		printf("Error reading file '%s'\n", filename);
> -	else {
> -		setenv_hex("filesize", size);
> -		printf("Done\n");
> +		*actread = i * PAGE_SIZE;
> +	} else {
> +		*actread = size;
>   	}
>
> +put_inode:
>   	ubifs_iput(inode);
>
>   out:
>   	ubi_close_volume(c->ubi);
>   	return err;
>   }
> +
> +/* Compat wrappers for common/cmd_ubifs.c */
> +int ubifs_load(char *filename, u32 addr, u32 size)
> +{
> +	loff_t actread;
> +	int err;
> +
> +	printf("Loading file '%s' to addr 0x%08x...\n", filename, addr);
> +
> +	err = ubifs_read(filename, (void *)addr, 0, size, &actread);
> +	if (err == 0) {
> +		setenv_hex("filesize", actread);
> +		printf("Done\n");
> +	}
> +
> +	return err;
> +}
> +
> +void uboot_ubifs_umount(void)
> +{
> +	if (ubifs_sb) {
> +		printf("Unmounting UBIFS volume %s!\n",
> +		       ((struct ubifs_info *)(ubifs_sb->s_fs_info))->vi.name);
> +		ubifs_umount(ubifs_sb->s_fs_info);
> +		ubifs_sb = NULL;
> +	}
> +}
> diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
> index a51b237..225965c 100644
> --- a/fs/ubifs/ubifs.h
> +++ b/fs/ubifs/ubifs.h
> @@ -34,6 +34,7 @@
>   #include <asm/atomic.h>
>   #include <asm-generic/atomic-long.h>
>   #include <ubi_uboot.h>
> +#include <ubifs_uboot.h>
>
>   #include <linux/ctype.h>
>   #include <linux/time.h>
> @@ -2379,11 +2380,6 @@ int ubifs_decompress(const void *buf, int len, void *out, int *out_len,
>   #include "key.h"
>
>   #ifdef __UBOOT__
> -/* these are used in cmd_ubifs.c */
> -int ubifs_init(void);
> -int uboot_ubifs_mount(char *vol_name);
>   void ubifs_umount(struct ubifs_info *c);
> -int ubifs_ls(char *dir_name);
> -int ubifs_load(char *filename, u32 addr, u32 size);
>   #endif
>   #endif /* !__UBIFS_H__ */
> diff --git a/include/ubifs_uboot.h b/include/ubifs_uboot.h
> new file mode 100644
> index 0000000..d10a909
> --- /dev/null
> +++ b/include/ubifs_uboot.h
> @@ -0,0 +1,29 @@
> +/*
> + * UBIFS u-boot wrapper functions header
> + *
> + * Copyright (C) 2006-2008 Nokia Corporation
> + *
> + * (C) Copyright 2008-2009
> + * Stefan Roese, DENX Software Engineering, sr at denx.de.
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + *
> + * Authors: Artem Bityutskiy (???????? ?????)
> + *          Adrian Hunter
> + */
> +
> +#ifndef __UBIFS_UBOOT_H__
> +#define __UBIFS_UBOOT_H__
> +
> +extern int ubifs_mounted;
> +
> +int ubifs_init(void);
> +int uboot_ubifs_mount(char *vol_name);
> +void uboot_ubifs_umount(void);
> +int ubifs_load(char *filename, u32 addr, u32 size);
> +
> +int ubifs_ls(const char *dir_name);
> +int ubifs_read(const char *filename, void *buf, loff_t offset,
> +	       loff_t size, loff_t *actread);
> +
> +#endif /* __UBIFS_UBOOT_H__ */
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany