On 01/09/2012 11:54 AM, uma.shan...@samsung.com wrote:
> From: uma.shankar <uma.shan...@samsung.com>
> 
> Signed-off-by: Uma Shankar <uma.shan...@samsung.com>
> Signed-off-by: Manjunatha C Achar <a.manjuna...@samsung.com>
> Signed-off-by: Iqbal Shareef <iqbal....@samsung.com>
> Signed-off-by: Hakgoo Lee <goodguy....@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to