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

Rob Herring
2012-03-27T15:10:04Z

On 01/09/2012 11:54 AM, uma.shankar at samsung.com wrote:
> From: uma.shankar <uma.shankar@samsung.com>
>
> Signed-off-by: Uma Shankar <uma.shankar@samsung.com>
> Signed-off-by: Manjunatha C Achar <a.manjunatha@samsung.com>
> Signed-off-by: Iqbal Shareef <iqbal.ams@samsung.com>
> Signed-off-by: Hakgoo Lee <goodguy.lee@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