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

On 01/09/2012 11:54 AM, uma.shankar@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