[U-Boot] [PATCH V4 1/2] ext4fs ls load support

[Rob Herring]

On 01/09/2012 11:54 AM, uma.shankar at samsung.com wrote:
> From: uma.shankar <uma.shankar at samsung.com>
> 
> Signed-off-by: Uma Shankar <uma.shankar at samsung.com>
> Signed-off-by: Manjunatha C Achar <a.manjunatha at samsung.com>
> Signed-off-by: Iqbal Shareef <iqbal.ams at samsung.com>
> Signed-off-by: Hakgoo Lee <goodguy.lee at samsung.com>
> ---
> Changes for v3:
>         - Copyright has been updated in respective files
>         - ext4fs has been made independant of ext2fs.c
>         - Fixed API namespace
>         - Removed endianness conversion API, used uboot defined API
>           for the same
>         - Fixed coding style issues
>         - Moved README.ext4 file into doc folder
> 
> Changes for v2:
>         - Code cleanup, changed comment style
>         - camel case removed, resolved code alignment issues
>         - memory allocation logic changed, removed busybox logic
>         - Modified ext4 load to remove grub dependency (GPLv3)
>         - Introduced new Config for ext4 write
> 
> Changes for v1:
>         - Removed checkpatch warnings and errors
>         - Moved common API's of ext2 and ext4 to one generic header file

snip

> +static int do_ext4_load(cmd_tbl_t *cmdtp, int flag, int argc,
> +						char *const argv[])
> +{
> +	char *filename = NULL;
> +	char *ep;
> +	int dev;
> +	unsigned long part = 1;
> +	ulong addr = 0;
> +	ulong part_length;
> +	int filelen;
> +	disk_partition_t info;
> +	struct ext_filesystem *fs;
> +	char buf[12];
> +	unsigned long count;
> +	const char *addr_str;
> +
> +	count = 0;
> +	addr = simple_strtoul(argv[3], NULL, 16);
> +	filename = getenv("bootfile");
> +	switch (argc) {
> +	case 3:
> +		addr_str = getenv("loadaddr");
> +		if (addr_str != NULL)
> +			addr = simple_strtoul(addr_str, NULL, 16);
> +		else
> +			addr = CONFIG_SYS_LOAD_ADDR;
> +
> +		break;
> +	case 4:
> +		break;
> +	case 5:
> +		filename = argv[4];
> +		break;
> +	case 6:
> +		filename = argv[4];
> +		count = simple_strtoul(argv[5], NULL, 16);
> +		break;
> +
> +	default:
> +		return cmd_usage(cmdtp);
> +	}
> +
> +	if (!filename) {
> +		puts("** No boot file defined **\n");
> +		return 1;
> +	}
> +
> +	dev = (int)simple_strtoul(argv[2], &ep, 16);
> +	ext4_dev_desc = get_dev(argv[1], dev);
> +	if (ext4_dev_desc == NULL) {
> +		printf("** Block device %s %d not supported\n", argv[1], dev);
> +		return 1;
> +	}
> +	if (init_fs(ext4_dev_desc))
> +		return 1;
> +
> +	fs = get_fs();

These functions pollute the name space and could be combined.

Or can't these 2 calls be rolled into...

> +	if (*ep) {
> +		if (*ep != ':') {
> +			puts("** Invalid boot device, use `dev[:part]' **\n");
> +			return 1;
> +		}
> +		part = simple_strtoul(++ep, NULL, 16);
> +	}
> +
> +	if (part != 0) {
> +		if (get_partition_info(fs->dev_desc, part, &info)) {
> +			printf("** Bad partition %lu **\n", part);
> +			return 1;
> +		}
> +
> +		if (strncmp((char *)info.type, BOOT_PART_TYPE,
> +			    strlen(BOOT_PART_TYPE)) != 0) {
> +			printf("** Invalid partition type \"%s\""
> +			       " (expect \"" BOOT_PART_TYPE "\")\n", info.type);
> +			return 1;
> +		}
> +		printf("Loading file \"%s\" "
> +		       "from %s device %d:%lu %s\n",
> +		       filename, argv[1], dev, part, info.name);
> +	} else {
> +		printf("Loading file \"%s\" from %s device %d\n",
> +		       filename, argv[1], dev);
> +	}
> +
> +	part_length = ext4fs_set_blk_dev(fs->dev_desc, part);

...this function?

> +	if (part_length == 0) {
> +		printf("**Bad partition - %s %d:%lu **\n", argv[1], dev, part);
> +		ext4fs_close();

You are leaking memory allocated in init_fs here.

> +		return 1;
> +	}
> +
> +	if (!ext4fs_mount(part_length)) {
> +		printf("** Bad ext2 partition or disk - %s %d:%lu **\n",
> +		       argv[1], dev, part);
> +		ext4fs_close();

and here...

> +		return 1;
> +	}
> +
> +	filelen = ext4fs_open(filename);
> +	if (filelen < 0) {
> +		printf("** File not found %s\n", filename);
> +		ext4fs_close();

and here...

> +		return 1;
> +	}
> +	if ((count < filelen) && (count != 0))
> +		filelen = count;
> +
> +	if (ext4fs_read((char *)addr, filelen) != filelen) {
> +		printf("** Unable to read \"%s\" from %s %d:%lu **\n",
> +		       filename, argv[1], dev, part);
> +		ext4fs_close();

and here...

Same comments apply in do_ext4_ls.

Rob