[U-Boot] [PATCH 0/8] EXT2 cleanup

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Marek Vasut]

Now I'm angry ...

So, I cleaned up the ext2 filesystem code a bit. I tried to separate the changes
to increase the reviewability.

Marek Vasut (8):
  EXT2: Indent cleanup of dev.c
  EXT2: Indent cleanup ext2fs.c
  EXT2: Rework ext2fs_blockgroup() function
  EXT2: Rework ext2fs_read_file()
  EXT2: Rework ext2fs_read_symlink()
  EXT2: Rework ext2fs_find_file1()
  EXT2: Rework ext2fs_iterate_dir()
  EXT2: Rework ext2fs_read_block()

 fs/ext2/dev.c    |   74 +++---
 fs/ext2/ext2fs.c |  775 +++++++++++++++++++++++++++---------------------------
 2 files changed, 416 insertions(+), 433 deletions(-)

-- 
1.7.10

[U-Boot] [PATCH 1/8] EXT2: Indent cleanup of dev.c

[Marek Vasut]

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/dev.c |   74 +++++++++++++++++++++++++++------------------------------
 1 file changed, 35 insertions(+), 39 deletions(-)

diff --git a/fs/ext2/dev.c b/fs/ext2/dev.c
index 315ff53..f8cb8b7 100644
--- a/fs/ext2/dev.c
+++ b/fs/ext2/dev.c
@@ -35,6 +35,7 @@ static disk_partition_t part_info;
 int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
 {
 	ext2fs_block_dev_desc = rbdd;
+	int ret;
 
 	if (part == 0) {
 		/* disk doesn't use partition table */
@@ -42,60 +43,53 @@ int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
 		part_info.size = rbdd->lba;
 		part_info.blksz = rbdd->blksz;
 	} else {
-		if (get_partition_info
-		    (ext2fs_block_dev_desc, part, &part_info)) {
+		ret = get_partition_info(ext2fs_block_dev_desc, part,
+						&part_info);
+		if (ret)
 			return 0;
-		}
 	}
+
 	return part_info.size;
 }
 
-
 int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(char, sec_buf, SECTOR_SIZE);
 	unsigned sectors;
+	unsigned val;
 
-	/*
-	 *  Check partition boundaries
-	 */
-	if ((sector < 0) ||
-	    ((sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS)) >=
-		part_info.size)) {
-		/* errnum = ERR_OUTSIDE_PART; */
-		printf(" ** %s read outside partition sector %d\n",
-		       __func__,
-		       sector);
+	/* Check partition boundaries */
+	val = sector + ((byte_offset + byte_len - 1) >> SECTOR_BITS);
+	if ((sector < 0) || (val >= part_info.size)) {
+		printf("EXT2: read outside partition %d\n", sector);
 		return 0;
 	}
 
-	/*
-	 *  Get the read to the beginning of a partition.
-	 */
+	/* Get the read to the beginning of a partition */
 	sector += byte_offset >> SECTOR_BITS;
 	byte_offset &= SECTOR_SIZE - 1;
 
 	debug(" <%d, %d, %d>\n", sector, byte_offset, byte_len);
 
 	if (ext2fs_block_dev_desc == NULL) {
-		printf(" ** %s Invalid Block Device Descriptor (NULL)\n",
-		       __func__);
+		printf("EXT2: Invalid Block Device Descriptor (NULL)\n");
 		return 0;
 	}
 
 	if (byte_offset != 0) {
 		/* read first part which isn't aligned with start of sector */
-		if (ext2fs_block_dev_desc->
-		    block_read(ext2fs_block_dev_desc->dev,
-			       part_info.start + sector, 1,
-			       (unsigned long *) sec_buf) != 1) {
-			printf(" ** %s read error **\n", __func__);
+		val = ext2fs_block_dev_desc->block_read(
+				ext2fs_block_dev_desc->dev,
+				part_info.start + sector, 1,
+				(unsigned long *)sec_buf);
+		if (val != 1) {
+			printf("EXT2: read error\n");
 			return 0;
 		}
-		memcpy(buf, sec_buf + byte_offset,
-		       min(SECTOR_SIZE - byte_offset, byte_len));
-		buf += min(SECTOR_SIZE - byte_offset, byte_len);
-		byte_len -= min(SECTOR_SIZE - byte_offset, byte_len);
+		val = min(SECTOR_SIZE - byte_offset, byte_len);
+		memcpy(buf, sec_buf + byte_offset, val);
+		buf += val;
+		byte_len -= val;
 		sector++;
 	}
 
@@ -103,12 +97,12 @@ int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 	sectors = byte_len / SECTOR_SIZE;
 
 	if (sectors > 0) {
-		if (ext2fs_block_dev_desc->block_read(
-			ext2fs_block_dev_desc->dev,
-			part_info.start + sector,
-			sectors,
-			(unsigned long *) buf) != sectors) {
-			printf(" ** %s read error - block\n", __func__);
+		val = ext2fs_block_dev_desc->block_read(
+				ext2fs_block_dev_desc->dev,
+				part_info.start + sector, sectors,
+				(unsigned long *)buf);
+		if (val != sectors) {
+			printf("EXT2: read error - block\n");
 			return 0;
 		}
 
@@ -119,14 +113,16 @@ int ext2fs_devread(int sector, int byte_offset, int byte_len, char *buf)
 
 	if (byte_len != 0) {
 		/* read rest of data which are not in whole sector */
-		if (ext2fs_block_dev_desc->
-		    block_read(ext2fs_block_dev_desc->dev,
-			       part_info.start + sector, 1,
-			       (unsigned long *) sec_buf) != 1) {
-			printf(" ** %s read error - last part\n", __func__);
+		val = ext2fs_block_dev_desc->block_read(
+				ext2fs_block_dev_desc->dev,
+				part_info.start + sector, 1,
+				(unsigned long *)sec_buf);
+		if (val != 1) {
+			printf("EXT2: read error - last part\n");
 			return 0;
 		}
 		memcpy(buf, sec_buf, byte_len);
 	}
+
 	return 1;
 }
-- 
1.7.10

[U-Boot] [PATCH 2/8] EXT2: Indent cleanup ext2fs.c

[Marek Vasut]

* Mostly cleanup problems reported by checkpatch.pl -f
* Minor tweaks where it simplified the code

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/ext2fs.c |  264 ++++++++++++++++++++++++++----------------------------
 1 file changed, 128 insertions(+), 136 deletions(-)

diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
index a4f3094..f9e9228 100644
--- a/fs/ext2/ext2fs.c
+++ b/fs/ext2/ext2fs.c
@@ -27,12 +27,11 @@
 #include <ext2fs.h>
 #include <ext_common.h>
 #include <malloc.h>
-#include <asm/byteorder.h>
+#include <linux/byteorder/generic.h>
 
-extern int ext2fs_devread (int sector, int byte_offset, int byte_len,
+extern int ext2fs_devread(int sector, int byte_offset, int byte_len,
 			   char *buf);
 
-
 struct ext2_data *ext2fs_root = NULL;
 struct ext2fs_node *ext2fs_file;
 int symlinknest = 0;
@@ -65,55 +64,55 @@ static int ext2fs_blockgroup
 
 }
 
-
-static int ext2fs_read_inode
-	(struct ext2_data *data, int ino, struct ext2_inode *inode) {
+static int ext2fs_read_inode(struct ext2_data *data, int ino,
+				struct ext2_inode *inode)
+{
 	struct ext2_block_group blkgrp;
 	struct ext2_sblock *sblock = &data->sblock;
-	int inodes_per_block;
-	int status;
-
 	unsigned int blkno;
 	unsigned int blkoff;
+	int status;
+	int inodes_per_block;
 
-#ifdef DEBUG
-	printf ("ext2fs read inode %d, inode_size %d\n", ino, inode_size);
-#endif
-	/* It is easier to calculate if the first inode is 0.  */
+	debug("EXT2: read inode %d, inode_size %d\n", ino, inode_size);
+
+	/* It is easier to calculate if the first inode is 0. */
 	ino--;
-	status = ext2fs_blockgroup (data, ino / __le32_to_cpu
-				    (sblock->inodes_per_group), &blkgrp);
-	if (status == 0) {
-		return (0);
-	}
+	status = ext2fs_blockgroup(data,
+			ino / __le32_to_cpu(sblock->inodes_per_group),
+			&blkgrp);
+	if (status == 0)
+		return 0;
 
 	inodes_per_block = EXT2_BLOCK_SIZE(data) / inode_size;
 
-	blkno = __le32_to_cpu (blkgrp.inode_table_id) +
-		(ino % __le32_to_cpu (sblock->inodes_per_group))
-		/ inodes_per_block;
+	blkno = __le32_to_cpu(blkgrp.inode_table_id) +
+			(ino % __le32_to_cpu(sblock->inodes_per_group)) /
+			inodes_per_block;
 	blkoff = (ino % inodes_per_block) * inode_size;
-#ifdef DEBUG
-	printf ("ext2fs read inode blkno %d blkoff %d\n", blkno, blkoff);
-#endif
-	/* Read the inode.  */
-	status = ext2fs_devread (blkno << LOG2_EXT2_BLOCK_SIZE (data), blkoff,
+
+	debug("EXT2: read inode blkno %d blkoff %d\n", blkno, blkoff);
+
+	/* Read the inode. */
+	status = ext2fs_devread(blkno << LOG2_EXT2_BLOCK_SIZE (data), blkoff,
 				 sizeof (struct ext2_inode), (char *) inode);
-	if (status == 0) {
-		return (0);
-	}
+	if (status == 0)
+		return 0;
 
-	return (1);
+	return 1;
 }
 
-
-void ext2fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot)
+static void ext2fs_free_node(struct ext2fs_node *node,
+				struct ext2fs_node *currroot)
 {
-	if ((node != &ext2fs_root->diropen) && (node != currroot)) {
-		free (node);
-	}
-}
+	if (node == &ext2fs_root->diropen)
+		return;
+
+	if (node == currroot)
+		return;
 
+	free(node);
+}
 
 static int ext2fs_read_block(struct ext2fs_node *node, int fileblock)
 {
@@ -490,7 +489,6 @@ static char *ext2fs_read_symlink(struct ext2fs_node *node)
 	return (symlink);
 }
 
-
 int ext2fs_find_file1
 	(const char *currpath, struct ext2fs_node *currroot,
 		struct ext2fs_node **currfound, int *foundtype)
@@ -595,172 +593,166 @@ int ext2fs_find_file1
 	return (-1);
 }
 
-
-int ext2fs_find_file
-	(const char *path, struct ext2fs_node *rootnode,
-	struct ext2fs_node **foundnode, int expecttype)
+static int ext2fs_find_file(const char *path, struct ext2fs_node *rootnode,
+			struct ext2fs_node **foundnode, int expecttype)
 {
 	int status;
 	int foundtype = FILETYPE_DIRECTORY;
 
-
 	symlinknest = 0;
-	if (!path) {
-		return (0);
-	}
+	if (!path)
+		return 0;
 
-	status = ext2fs_find_file1 (path, rootnode, foundnode, &foundtype);
-	if (status == 0) {
-		return (0);
-	}
-	/* Check if the node that was found was of the expected type.  */
-	if ((expecttype == FILETYPE_REG) && (foundtype != expecttype)) {
-		return (0);
-	} else if ((expecttype == FILETYPE_DIRECTORY)
-		   && (foundtype != expecttype)) {
-		return (0);
+	status = ext2fs_find_file1(path, rootnode, foundnode, &foundtype);
+	if (status == 0)
+		return 0;
+
+	/* Check if the node that was found was of the expected type. */
+	if (foundtype != expecttype) {
+		if (expecttype == FILETYPE_REG)
+			return 0;
+		if (expecttype == FILETYPE_DIRECTORY)
+			return 0;
 	}
-	return (1);
-}
 
+	return 1;
+}
 
-int ext2fs_ls (const char *dirname) {
+int ext2fs_ls(const char *dirname)
+{
 	struct ext2fs_node *dirnode;
 	int status;
 
-	if (ext2fs_root == NULL) {
-		return (0);
-	}
+	if (ext2fs_root == NULL)
+		return 0;
 
-	status = ext2fs_find_file (dirname, &ext2fs_root->diropen, &dirnode,
-				   FILETYPE_DIRECTORY);
+	status = ext2fs_find_file(dirname, &ext2fs_root->diropen, &dirnode,
+					FILETYPE_DIRECTORY);
 	if (status != 1) {
-		printf ("** Can not find directory. **\n");
-		return (1);
+		printf("EXT2: Can not find directory!\n");
+		return 1;
 	}
-	ext2fs_iterate_dir (dirnode, NULL, NULL, NULL);
-	ext2fs_free_node (dirnode, &ext2fs_root->diropen);
-	return (0);
-}
 
+	ext2fs_iterate_dir(dirnode, NULL, NULL, NULL);
+	ext2fs_free_node(dirnode, &ext2fs_root->diropen);
+
+	return 0;
+}
 
-int ext2fs_open (const char *filename) {
+int ext2fs_open(const char *filename)
+{
 	struct ext2fs_node *fdiro = NULL;
 	int status;
-	int len;
 
-	if (ext2fs_root == NULL) {
-		return (-1);
-	}
+	if (ext2fs_root == NULL)
+		return -1;
+
 	ext2fs_file = NULL;
-	status = ext2fs_find_file (filename, &ext2fs_root->diropen, &fdiro,
-				   FILETYPE_REG);
-	if (status == 0) {
+	status = ext2fs_find_file(filename, &ext2fs_root->diropen, &fdiro,
+					FILETYPE_REG);
+	if (status == 0)
 		goto fail;
-	}
+
 	if (!fdiro->inode_read) {
-		status = ext2fs_read_inode (fdiro->data, fdiro->ino,
-					    &fdiro->inode);
-		if (status == 0) {
+		status = ext2fs_read_inode(fdiro->data, fdiro->ino,
+						&fdiro->inode);
+		if (status == 0)
 			goto fail;
-		}
 	}
-	len = __le32_to_cpu (fdiro->inode.size);
+
 	ext2fs_file = fdiro;
-	return (len);
+
+	return __le32_to_cpu(fdiro->inode.size);
 
 fail:
-	ext2fs_free_node (fdiro, &ext2fs_root->diropen);
-	return (-1);
+	ext2fs_free_node(fdiro, &ext2fs_root->diropen);
+	return -1;
 }
 
-
 int ext2fs_close(void)
 {
-	if ((ext2fs_file != NULL) && (ext2fs_root != NULL)) {
-		ext2fs_free_node (ext2fs_file, &ext2fs_root->diropen);
-		ext2fs_file = NULL;
-	}
 	if (ext2fs_root != NULL) {
-		free (ext2fs_root);
+		if (ext2fs_file != NULL) {
+			ext2fs_free_node(ext2fs_file, &ext2fs_root->diropen);
+			ext2fs_file = NULL;
+		}
+
+		free(ext2fs_root);
 		ext2fs_root = NULL;
 	}
+
 	if (indir1_block != NULL) {
-		free (indir1_block);
+		free(indir1_block);
 		indir1_block = NULL;
 		indir1_size = 0;
 		indir1_blkno = -1;
 	}
+
 	if (indir2_block != NULL) {
-		free (indir2_block);
+		free(indir2_block);
 		indir2_block = NULL;
 		indir2_size = 0;
 		indir2_blkno = -1;
 	}
-	return (0);
-}
 
+	return 0;
+}
 
-int ext2fs_read (char *buf, unsigned len) {
-	int status;
-
-	if (ext2fs_root == NULL) {
-		return (0);
-	}
+int ext2fs_read(char *buf, unsigned len)
+{
+	if (ext2fs_root == NULL)
+		return 0;
 
-	if (ext2fs_file == NULL) {
-		return (0);
-	}
+	if (ext2fs_file == NULL)
+		return 0;
 
-	status = ext2fs_read_file (ext2fs_file, 0, len, buf);
-	return (status);
+	return ext2fs_read_file(ext2fs_file, 0, len, buf);
 }
 
-
-int ext2fs_mount (unsigned part_length) {
+int ext2fs_mount(unsigned part_length)
+{
 	struct ext2_data *data;
 	int status;
 
-	data = malloc (sizeof (struct ext2_data));
-	if (!data) {
-		return (0);
-	}
-	/* Read the superblock.  */
-	status = ext2fs_devread (1 * 2, 0, sizeof (struct ext2_sblock),
-				 (char *) &data->sblock);
-	if (status == 0) {
+	data = malloc(sizeof(struct ext2_data));
+	if (!data)
+		return 0;
+
+	/* Read the superblock. */
+	status = ext2fs_devread(1 * 2, 0, sizeof(struct ext2_sblock),
+				(char *)&data->sblock);
+
+	if (status == 0)
 		goto fail;
-	}
-	/* Make sure this is an ext2 filesystem.  */
-	if (__le16_to_cpu (data->sblock.magic) != EXT2_MAGIC) {
+
+	/* Make sure this is an ext2 filesystem. */
+	if (__le16_to_cpu(data->sblock.magic) != EXT2_MAGIC)
 		goto fail;
-	}
-	if (__le32_to_cpu(data->sblock.revision_level == 0)) {
+
+	if (__le32_to_cpu(data->sblock.revision_level == 0))
 		inode_size = 128;
-	} else {
+	else
 		inode_size = __le16_to_cpu(data->sblock.inode_size);
-	}
-#ifdef DEBUG
-	printf("EXT2 rev %d, inode_size %d\n",
-			__le32_to_cpu(data->sblock.revision_level), inode_size);
-#endif
+
+	debug("EXT2: rev %d, inode_size %d\n",
+		__le32_to_cpu(data->sblock.revision_level), inode_size);
+
 	data->diropen.data = data;
 	data->diropen.ino = 2;
 	data->diropen.inode_read = 1;
 	data->inode = &data->diropen.inode;
 
-	status = ext2fs_read_inode (data, 2, data->inode);
-	if (status == 0) {
+	status = ext2fs_read_inode(data, 2, data->inode);
+	if (status == 0)
 		goto fail;
-	}
 
 	ext2fs_root = data;
 
-	return (1);
-
+	return 1;
 fail:
-	printf ("Failed to mount ext2 filesystem...\n");
-	free (data);
+	printf("EXT2: Failed to mount ext2 filesystem!\n");
+	free(data);
 	ext2fs_root = NULL;
-	return (0);
+
+	return 0;
 }
-- 
1.7.10

[U-Boot] [PATCH 3/8] EXT2: Rework ext2fs_blockgroup() function

[Marek Vasut]

Rework the function, constify every possible component.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/ext2fs.c |   31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
index f9e9228..69fe1d1 100644
--- a/fs/ext2/ext2fs.c
+++ b/fs/ext2/ext2fs.c
@@ -43,25 +43,22 @@ int indir2_size = 0;
 int indir2_blkno = -1;
 static unsigned int inode_size;
 
-
-static int ext2fs_blockgroup
-	(struct ext2_data *data, int group, struct ext2_block_group *blkgrp) {
-	unsigned int blkno;
-	unsigned int blkoff;
-	unsigned int desc_per_blk;
-
-	desc_per_blk = EXT2_BLOCK_SIZE(data) / sizeof(struct ext2_block_group);
-
-	blkno = __le32_to_cpu(data->sblock.first_data_block) + 1 +
-	group / desc_per_blk;
-	blkoff = (group % desc_per_blk) * sizeof(struct ext2_block_group);
-#ifdef DEBUG
-	printf ("ext2fs read %d group descriptor (blkno %d blkoff %d)\n",
+static int ext2fs_blockgroup(struct ext2_data *data, int group,
+				struct ext2_block_group *blkgrp)
+{
+	struct ext2_sblock *sb = &data->sblock;
+	const unsigned int blk_sz = EXT2_BLOCK_SIZE(data);
+	const unsigned int grp_sz = sizeof(struct ext2_block_group);
+	const unsigned int desc_per_blk = blk_sz / grp_sz;
+	const unsigned int blkoff = (group % desc_per_blk) * grp_sz;
+	const unsigned int first_block = __le32_to_cpu(sb->first_data_block);
+	const unsigned int blkno = first_block + 1 + group / desc_per_blk;
+
+	debug("EXT2: read %d group descriptor (blkno %d blkoff %d)\n",
 		group, blkno, blkoff);
-#endif
-	return (ext2fs_devread (blkno << LOG2_EXT2_BLOCK_SIZE(data),
-		blkoff, sizeof(struct ext2_block_group), (char *)blkgrp));
 
+	return ext2fs_devread(blkno << LOG2_EXT2_BLOCK_SIZE(data),
+				blkoff, grp_sz, (char *)blkgrp);
 }
 
 static int ext2fs_read_inode(struct ext2_data *data, int ino,
-- 
1.7.10

[U-Boot] [PATCH 4/8] EXT2: Rework ext2fs_read_file()

[Marek Vasut]

* Fix indentation
* Move all definitions of variables to the begining of the function

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/ext2fs.c |   65 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
index 69fe1d1..0d54ae6 100644
--- a/fs/ext2/ext2fs.c
+++ b/fs/ext2/ext2fs.c
@@ -256,66 +256,69 @@ static int ext2fs_read_block(struct ext2fs_node *node, int fileblock)
 	return (blknr);
 }
 
-
-int ext2fs_read_file
-	(struct ext2fs_node *node, int pos, unsigned int len, char *buf)
+static int ext2fs_read_file(struct ext2fs_node *node, int pos,
+				unsigned int len, char *buf)
 {
+	const int log2blocksize = LOG2_EXT2_BLOCK_SIZE (node->data);
+	const int blocksize = 1 << (log2blocksize + DISK_SECTOR_BITS);
+	const unsigned int filesize = __le32_to_cpu(node->inode.size);
+
 	int i;
 	int blockcnt;
-	int log2blocksize = LOG2_EXT2_BLOCK_SIZE (node->data);
-	int blocksize = 1 << (log2blocksize + DISK_SECTOR_BITS);
-	unsigned int filesize = __le32_to_cpu(node->inode.size);
+	int status;
+	int blknr;
+	int blockoff;
+	int blockend;
+	int skipfirst;
 
-	/* Adjust len so it we can't read past the end of the file.  */
-	if (len > filesize) {
+	/* Adjust len so it we can't read past the end of the file. */
+	if (len > filesize)
 		len = filesize;
-	}
+
 	blockcnt = ((len + pos) + blocksize - 1) / blocksize;
 
 	for (i = pos / blocksize; i < blockcnt; i++) {
-		int blknr;
-		int blockoff = pos % blocksize;
-		int blockend = blocksize;
-
-		int skipfirst = 0;
+		blockoff = pos % blocksize;
+		blockend = blocksize;
+		skipfirst = 0;
 
 		blknr = ext2fs_read_block (node, i);
-		if (blknr < 0) {
-			return (-1);
-		}
+		if (blknr < 0)
+			return -1;
+
 		blknr = blknr << log2blocksize;
 
 		/* Last block.  */
 		if (i == blockcnt - 1) {
 			blockend = (len + pos) % blocksize;
 
-			/* The last portion is exactly blocksize.  */
-			if (!blockend) {
+			/* The last portion is exactly blocksize. */
+			if (!blockend)
 				blockend = blocksize;
-			}
 		}
 
-		/* First block.  */
+		/* First block. */
 		if (i == pos / blocksize) {
 			skipfirst = blockoff;
 			blockend -= skipfirst;
 		}
 
-		/* If the block number is 0 this block is not stored on disk but
-		   is zero filled instead.  */
+		/*
+		 * If the block number is 0 this block is not stored on disk
+		 * but is zero filled instead.
+		 */
 		if (blknr) {
-			int status;
-
-			status = ext2fs_devread (blknr, skipfirst, blockend, buf);
-			if (status == 0) {
-				return (-1);
-			}
+			status = ext2fs_devread(blknr, skipfirst, blockend, buf);
+			if (status == 0)
+				return -1;
 		} else {
-			memset (buf, 0, blocksize - skipfirst);
+			memset(buf, 0, blocksize - skipfirst);
 		}
+
 		buf += blocksize - skipfirst;
 	}
-	return (len);
+
+	return len;
 }
 
 int ext2fs_iterate_dir(struct ext2fs_node *dir, char *name,
-- 
1.7.10

[U-Boot] [PATCH 5/8] EXT2: Rework ext2fs_read_symlink()

[Marek Vasut]

* Fix indent
* Read inode size only once, as it can be reused

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/ext2fs.c |   45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
index 0d54ae6..22cc9c2 100644
--- a/fs/ext2/ext2fs.c
+++ b/fs/ext2/ext2fs.c
@@ -458,35 +458,36 @@ static char *ext2fs_read_symlink(struct ext2fs_node *node)
 	char *symlink;
 	struct ext2fs_node *diro = node;
 	int status;
+	uint32_t size;
 
 	if (!diro->inode_read) {
-		status = ext2fs_read_inode (diro->data, diro->ino,
-					    &diro->inode);
-		if (status == 0) {
-			return (0);
-		}
-	}
-	symlink = malloc (__le32_to_cpu (diro->inode.size) + 1);
-	if (!symlink) {
-		return (0);
+		status = ext2fs_read_inode(diro->data, diro->ino, &diro->inode);
+		if (status == 0)
+			return 0;
 	}
-	/* If the filesize of the symlink is bigger than
-	   60 the symlink is stored in a separate block,
-	   otherwise it is stored in the inode.  */
-	if (__le32_to_cpu (diro->inode.size) <= 60) {
-		strncpy (symlink, diro->inode.b.symlink,
-			 __le32_to_cpu (diro->inode.size));
+
+	size = __le32_to_cpu(diro->inode.size);
+	symlink = malloc(size + 1);
+	if (!symlink)
+		return 0;
+
+	/*
+	 * If the filesize of the symlink is bigger than 60 the symlink is
+	 * stored in a separate block, otherwise it is stored in the inode.
+	 */
+	if (size <= 60) {
+		strncpy(symlink, diro->inode.b.symlink, size);
 	} else {
-		status = ext2fs_read_file (diro, 0,
-					   __le32_to_cpu (diro->inode.size),
-					   symlink);
+		status = ext2fs_read_file(diro, 0, size, symlink);
 		if (status == 0) {
-			free (symlink);
-			return (0);
+			free(symlink);
+			return 0;
 		}
 	}
-	symlink[__le32_to_cpu (diro->inode.size)] = '\0';
-	return (symlink);
+
+	symlink[size] = '\0';
+
+	return symlink;
 }
 
 int ext2fs_find_file1
-- 
1.7.10

[U-Boot] [PATCH 6/8] EXT2: Rework ext2fs_find_file1()

[Marek Vasut]

* Fix indent
* Pull variables defined deep in the code at the begining

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/ext2fs.c |  101 +++++++++++++++++++++++++++---------------------------
 1 file changed, 50 insertions(+), 51 deletions(-)

diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
index 22cc9c2..10745f6 100644
--- a/fs/ext2/ext2fs.c
+++ b/fs/ext2/ext2fs.c
@@ -490,108 +490,107 @@ static char *ext2fs_read_symlink(struct ext2fs_node *node)
 	return symlink;
 }
 
-int ext2fs_find_file1
-	(const char *currpath, struct ext2fs_node *currroot,
-		struct ext2fs_node **currfound, int *foundtype)
+static int ext2fs_find_file1(const char *currpath, struct ext2fs_node *currroot,
+				struct ext2fs_node **currfound, int *foundtype)
 {
-	char fpath[strlen (currpath) + 1];
+	char fpath[strlen(currpath) + 1];
 	char *name = fpath;
 	char *next;
 	int status;
 	int type = FILETYPE_DIRECTORY;
+	int found;
+	char *symlink;
 	struct ext2fs_node *currnode = currroot;
 	struct ext2fs_node *oldnode = currroot;
 
-	strncpy (fpath, currpath, strlen (currpath) + 1);
+	strncpy(fpath, currpath, strlen(currpath) + 1);
 
-	/* Remove all leading slashes.  */
-	while (*name == '/') {
+	/* Remove all leading slashes. */
+	while (*name == '/')
 		name++;
-	}
+
 	if (!*name) {
 		*currfound = currnode;
-		return (1);
+		return 1;
 	}
 
 	for (;;) {
-		int found;
-
-		/* Extract the actual part from the pathname.  */
-		next = strchr (name, '/');
+		/* Extract the actual part from the pathname. */
+		next = strchr(name, '/');
 		if (next) {
-			/* Remove all leading slashes.  */
-			while (*next == '/') {
+			/* Remove all leading slashes. */
+			while (*next == '/')
 				*(next++) = '\0';
-			}
 		}
 
-		/* At this point it is expected that the current node is a directory, check if this is true.  */
+		/*
+		 * At this point it is expected that the current node is a
+		 * directory, check if this is true.
+		 */
 		if (type != FILETYPE_DIRECTORY) {
-			ext2fs_free_node (currnode, currroot);
-			return (0);
+			ext2fs_free_node(currnode, currroot);
+			return 0;
 		}
 
 		oldnode = currnode;
 
-		/* Iterate over the directory.  */
-		found = ext2fs_iterate_dir (currnode, name, &currnode, &type);
-		if (found == 0) {
-			return (0);
-		}
-		if (found == -1) {
+		/* Iterate over the directory. */
+		found = ext2fs_iterate_dir(currnode, name, &currnode, &type);
+		if (found == 0)
+			return 0;
+
+		if (found == -1)
 			break;
-		}
 
-		/* Read in the symlink and follow it.  */
+		/* Read in the symlink and follow it. */
 		if (type == FILETYPE_SYMLINK) {
-			char *symlink;
+			symlink = NULL;
 
-			/* Test if the symlink does not loop.  */
+			/* Test if the symlink does not loop. */
 			if (++symlinknest == 8) {
-				ext2fs_free_node (currnode, currroot);
-				ext2fs_free_node (oldnode, currroot);
-				return (0);
+				ext2fs_free_node(currnode, currroot);
+				ext2fs_free_node(oldnode, currroot);
+				return 0;
 			}
 
-			symlink = ext2fs_read_symlink (currnode);
-			ext2fs_free_node (currnode, currroot);
+			symlink = ext2fs_read_symlink(currnode);
+			ext2fs_free_node(currnode, currroot);
 
 			if (!symlink) {
-				ext2fs_free_node (oldnode, currroot);
-				return (0);
+				ext2fs_free_node(oldnode, currroot);
+				return 0;
 			}
-#ifdef DEBUG
-			printf ("Got symlink >%s<\n", symlink);
-#endif /* of DEBUG */
-			/* The symlink is an absolute path, go back to the root inode.  */
+
+			debug("EXT2: Got symlink >%s<\n", symlink);
+
 			if (symlink[0] == '/') {
-				ext2fs_free_node (oldnode, currroot);
+				ext2fs_free_node(oldnode, currroot);
 				oldnode = &ext2fs_root->diropen;
 			}
 
-			/* Lookup the node the symlink points to.  */
-			status = ext2fs_find_file1 (symlink, oldnode,
-						    &currnode, &type);
+			/* Lookup the node the symlink points to. */
+			status = ext2fs_find_file1(symlink, oldnode,
+						   &currnode, &type);
 
-			free (symlink);
+			free(symlink);
 
 			if (status == 0) {
-				ext2fs_free_node (oldnode, currroot);
-				return (0);
+				ext2fs_free_node(oldnode, currroot);
+				return 0;
 			}
 		}
 
-		ext2fs_free_node (oldnode, currroot);
+		ext2fs_free_node(oldnode, currroot);
 
-		/* Found the node!  */
+		/* Found the node! */
 		if (!next || *next == '\0') {
 			*currfound = currnode;
 			*foundtype = type;
-			return (1);
+			return 1;
 		}
 		name = next;
 	}
-	return (-1);
+	return -1;
 }
 
 static int ext2fs_find_file(const char *path, struct ext2fs_node *rootnode,
-- 
1.7.10

[U-Boot] [PATCH 7/8] EXT2: Rework ext2fs_iterate_dir()

[Marek Vasut]

* Indent cleanup
* Pull most variables defined inside the code to the begining
  (note: there are variables marked with "FIXME" now, those will be
         processed in subsequent patch)

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/ext2fs.c |  139 +++++++++++++++++++++++++++---------------------------
 1 file changed, 69 insertions(+), 70 deletions(-)

diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
index 10745f6..d55c8c5 100644
--- a/fs/ext2/ext2fs.c
+++ b/fs/ext2/ext2fs.c
@@ -322,135 +322,134 @@ static int ext2fs_read_file(struct ext2fs_node *node, int pos,
 }
 
 int ext2fs_iterate_dir(struct ext2fs_node *dir, char *name,
-					struct ext2fs_node **fnode, int *ftype)
+			struct ext2fs_node **fnode, int *ftype)
 {
 	unsigned int fpos = 0;
 	int status;
-	struct ext2fs_node *diro = (struct ext2fs_node *) dir;
+	int type;
+	struct ext2fs_node *diro = (struct ext2fs_node *)dir;
+	struct ext2fs_node *fdiro;
+	struct ext2_dirent dirent;
 
-#ifdef DEBUG
 	if (name != NULL)
-		printf ("Iterate dir %s\n", name);
-#endif /* of DEBUG */
+		debug("EXT2: Iterate dir %s\n", name);
+
 	if (!diro->inode_read) {
-		status = ext2fs_read_inode (diro->data, diro->ino,
-					    &diro->inode);
-		if (status == 0) {
-			return (0);
-		}
+		status = ext2fs_read_inode(diro->data, diro->ino, &diro->inode);
+		if (status == 0)
+			return 0;
 	}
-	/* Search the file.  */
-	while (fpos < __le32_to_cpu (diro->inode.size)) {
-		struct ext2_dirent dirent;
-
-		status = ext2fs_read_file (diro, fpos,
-					   sizeof (struct ext2_dirent),
-					   (char *) &dirent);
-		if (status < 1) {
-			return (0);
-		}
+
+	/* Search the file. */
+	while (fpos < __le32_to_cpu(diro->inode.size)) {
+		status = ext2fs_read_file(diro, fpos,
+					  sizeof(struct ext2_dirent),
+					  (char *)&dirent);
+		if (status < 1)
+			return 0;
+
 		if (dirent.namelen != 0) {
-			char filename[dirent.namelen + 1];
-			struct ext2fs_node *fdiro;
-			int type = FILETYPE_UNKNOWN;
-
-			status = ext2fs_read_file (diro,
-						   fpos + sizeof (struct ext2_dirent),
-						   dirent.namelen, filename);
-			if (status < 1) {
-				return (0);
-			}
-			fdiro = malloc (sizeof (struct ext2fs_node));
-			if (!fdiro) {
-				return (0);
-			}
+			char filename[dirent.namelen + 1];	/* FIXME */
+			type = FILETYPE_UNKNOWN;
+			fdiro = NULL;
+
+			status = ext2fs_read_file(diro,
+						  fpos +
+						  sizeof(struct ext2_dirent),
+						  dirent.namelen, filename);
+			if (status < 1)
+				return 0;
+
+			fdiro = malloc(sizeof(struct ext2fs_node));
+			if (!fdiro)
+				return 0;
 
 			fdiro->data = diro->data;
-			fdiro->ino = __le32_to_cpu (dirent.inode);
+			fdiro->ino = __le32_to_cpu(dirent.inode);
 
 			filename[dirent.namelen] = '\0';
 
 			if (dirent.filetype != FILETYPE_UNKNOWN) {
 				fdiro->inode_read = 0;
 
-				if (dirent.filetype == FILETYPE_DIRECTORY) {
+				if (dirent.filetype == FILETYPE_DIRECTORY)
 					type = FILETYPE_DIRECTORY;
-				} else if (dirent.filetype ==
-					   FILETYPE_SYMLINK) {
+				else if (dirent.filetype == FILETYPE_SYMLINK)
 					type = FILETYPE_SYMLINK;
-				} else if (dirent.filetype == FILETYPE_REG) {
+				else if (dirent.filetype == FILETYPE_REG)
 					type = FILETYPE_REG;
-				}
 			} else {
-				/* The filetype can not be read from the dirent, get it from inode */
-
-				status = ext2fs_read_inode (diro->data,
-							    __le32_to_cpu(dirent.inode),
-							    &fdiro->inode);
+				/*
+				 * The filetype can not be read from the dirent,
+				 * get it from inode.
+				 */
+				status = ext2fs_read_inode(diro->data,
+						   __le32_to_cpu(dirent.inode),
+						   &fdiro->inode);
 				if (status == 0) {
-					free (fdiro);
-					return (0);
+					free(fdiro);
+					return 0;
 				}
 				fdiro->inode_read = 1;
 
-				if ((__le16_to_cpu (fdiro->inode.mode) &
+				if ((__le16_to_cpu(fdiro->inode.mode) &
 				     FILETYPE_INO_MASK) ==
 				    FILETYPE_INO_DIRECTORY) {
 					type = FILETYPE_DIRECTORY;
-				} else if ((__le16_to_cpu (fdiro->inode.mode)
+				} else if ((__le16_to_cpu(fdiro->inode.mode)
 					    & FILETYPE_INO_MASK) ==
 					   FILETYPE_INO_SYMLINK) {
 					type = FILETYPE_SYMLINK;
-				} else if ((__le16_to_cpu (fdiro->inode.mode)
+				} else if ((__le16_to_cpu(fdiro->inode.mode)
 					    & FILETYPE_INO_MASK) ==
 					   FILETYPE_INO_REG) {
 					type = FILETYPE_REG;
 				}
 			}
-#ifdef DEBUG
-			printf ("iterate >%s<\n", filename);
-#endif /* of DEBUG */
+
+			debug("EXT2: iterate >%s<\n", filename);
+
 			if ((name != NULL) && (fnode != NULL)
 			    && (ftype != NULL)) {
-				if (strcmp (filename, name) == 0) {
+				if (strcmp(filename, name) == 0) {
 					*ftype = type;
 					*fnode = fdiro;
-					return (1);
+					return 1;
 				}
 			} else {
 				if (fdiro->inode_read == 0) {
-					status = ext2fs_read_inode (diro->data,
-							    __le32_to_cpu (dirent.inode),
-							    &fdiro->inode);
+					status = ext2fs_read_inode(diro->data,
+							 __le32_to_cpu(dirent.inode),
+							 &fdiro->inode);
 					if (status == 0) {
-						free (fdiro);
-						return (0);
+						free(fdiro);
+						return 0;
 					}
 					fdiro->inode_read = 1;
 				}
 				switch (type) {
 				case FILETYPE_DIRECTORY:
-					printf ("<DIR> ");
+					printf("<DIR> ");
 					break;
 				case FILETYPE_SYMLINK:
-					printf ("<SYM> ");
+					printf("<SYM> ");
 					break;
 				case FILETYPE_REG:
-					printf ("      ");
+					printf("      ");
 					break;
 				default:
-					printf ("< ? > ");
+					printf("< ? > ");
 					break;
 				}
-				printf ("%10d %s\n",
-					__le32_to_cpu (fdiro->inode.size),
-					filename);
+				printf("%10d %s\n",
+				       __le32_to_cpu(fdiro->inode.size),
+				       filename);
 			}
-			free (fdiro);
+			free(fdiro);
 		}
-		fpos += __le16_to_cpu (dirent.direntlen);
+		fpos += __le16_to_cpu(dirent.direntlen);
 	}
-	return (0);
+	return 0;
 }
 
 static char *ext2fs_read_symlink(struct ext2fs_node *node)
-- 
1.7.10

[U-Boot] [PATCH 8/8] EXT2: Rework ext2fs_read_block()

[Marek Vasut]

Fix indentation complains from checkpatch.pl

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 fs/ext2/ext2fs.c |  130 ++++++++++++++++++++++++++----------------------------
 1 file changed, 63 insertions(+), 67 deletions(-)

diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
index d55c8c5..5bb8851 100644
--- a/fs/ext2/ext2fs.c
+++ b/fs/ext2/ext2fs.c
@@ -116,68 +116,64 @@ static int ext2fs_read_block(struct ext2fs_node *node, int fileblock)
 	struct ext2_data *data = node->data;
 	struct ext2_inode *inode = &node->inode;
 	int blknr;
-	int blksz = EXT2_BLOCK_SIZE (data);
-	int log2_blksz = LOG2_EXT2_BLOCK_SIZE (data);
+	int blksz = EXT2_BLOCK_SIZE(data);
+	int log2_blksz = LOG2_EXT2_BLOCK_SIZE(data);
+	int size;
 	int status;
 
-	/* Direct blocks.  */
+	/* Direct blocks. */
 	if (fileblock < INDIRECT_BLOCKS) {
-		blknr = __le32_to_cpu (inode->b.blocks.dir_blocks[fileblock]);
-	}
-	/* Indirect.  */
-	else if (fileblock < (INDIRECT_BLOCKS + (blksz / 4))) {
+		blknr = __le32_to_cpu(inode->b.blocks.dir_blocks[fileblock]);
+	/* Indirect. */
+	} else if (fileblock < (INDIRECT_BLOCKS + (blksz / 4))) {
 		if (indir1_block == NULL) {
-			indir1_block = (uint32_t *) memalign(ARCH_DMA_MINALIGN,
+			indir1_block = (uint32_t *)memalign(ARCH_DMA_MINALIGN,
 							     blksz);
 			if (indir1_block == NULL) {
-				printf ("** ext2fs read block (indir 1) malloc failed. **\n");
-				return (-1);
+				printf("EXT2: read block (indir 1) malloc failed!\n");
+				return -1;
 			}
 			indir1_size = blksz;
 			indir1_blkno = -1;
 		}
+
 		if (blksz != indir1_size) {
-			free (indir1_block);
+			free(indir1_block);
 			indir1_block = NULL;
 			indir1_size = 0;
 			indir1_blkno = -1;
-			indir1_block = (uint32_t *) memalign(ARCH_DMA_MINALIGN,
+			indir1_block = (uint32_t *)memalign(ARCH_DMA_MINALIGN,
 							     blksz);
 			if (indir1_block == NULL) {
-				printf ("** ext2fs read block (indir 1) malloc failed. **\n");
-				return (-1);
+				printf("EXT2: read block (indir 1) malloc failed!\n");
+				return -1;
 			}
 			indir1_size = blksz;
 		}
-		if ((__le32_to_cpu (inode->b.blocks.indir_block) <<
-		     log2_blksz) != indir1_blkno) {
-			status = ext2fs_devread (__le32_to_cpu(inode->b.blocks.indir_block) << log2_blksz,
-						 0, blksz,
-						 (char *) indir1_block);
+
+		size = __le32_to_cpu(inode->b.blocks.indir_block) << log2_blksz;
+		if (size != indir1_blkno) {
+			status = ext2fs_devread(size, 0, blksz,
+						(char *)indir1_block);
 			if (status == 0) {
-				printf ("** ext2fs read block (indir 1) failed. **\n");
-				return (0);
+				printf("EXT2: read block (indir 1) failed!\n");
+				return 0;
 			}
-			indir1_blkno =
-				__le32_to_cpu (inode->b.blocks.
-					       indir_block) << log2_blksz;
+			indir1_blkno = size;
 		}
-		blknr = __le32_to_cpu (indir1_block
-				       [fileblock - INDIRECT_BLOCKS]);
-	}
-	/* Double indirect.  */
-	else if (fileblock <
-		 (INDIRECT_BLOCKS + (blksz / 4 * (blksz / 4 + 1)))) {
+		blknr =__le32_to_cpu(indir1_block[fileblock - INDIRECT_BLOCKS]);
+	/* Double indirect. */
+	} else
+	if (fileblock < (INDIRECT_BLOCKS + (blksz / 4 * (blksz / 4 + 1)))) {
 		unsigned int perblock = blksz / 4;
-		unsigned int rblock = fileblock - (INDIRECT_BLOCKS
-						   + blksz / 4);
+		unsigned int rblock = fileblock - (INDIRECT_BLOCKS + blksz / 4);
 
 		if (indir1_block == NULL) {
-			indir1_block = (uint32_t *) memalign(ARCH_DMA_MINALIGN,
+			indir1_block = (uint32_t *)memalign(ARCH_DMA_MINALIGN,
 							     blksz);
 			if (indir1_block == NULL) {
-				printf ("** ext2fs read block (indir 2 1) malloc failed. **\n");
-				return (-1);
+				printf ("EXT2: read block (indir 2 1) malloc failed!\n");
+				return -1;
 			}
 			indir1_size = blksz;
 			indir1_blkno = -1;
@@ -190,72 +186,72 @@ static int ext2fs_read_block(struct ext2fs_node *node, int fileblock)
 			indir1_block = (uint32_t *) memalign(ARCH_DMA_MINALIGN,
 							     blksz);
 			if (indir1_block == NULL) {
-				printf ("** ext2fs read block (indir 2 1) malloc failed. **\n");
-				return (-1);
+				printf ("** ext2fs read block (indir 2 1) malloc failed!\n");
+				return -1;
 			}
 			indir1_size = blksz;
 		}
-		if ((__le32_to_cpu (inode->b.blocks.double_indir_block) <<
-		     log2_blksz) != indir1_blkno) {
-			status = ext2fs_devread (__le32_to_cpu(inode->b.blocks.double_indir_block) << log2_blksz,
-						0, blksz,
-						(char *) indir1_block);
+
+		size = __le32_to_cpu (inode->b.blocks.double_indir_block) << log2_blksz;
+		if (size != indir1_blkno) {
+			status = ext2fs_devread (size, 0, blksz,
+						(char *)indir1_block);
 			if (status == 0) {
-				printf ("** ext2fs read block (indir 2 1) failed. **\n");
-				return (-1);
+				printf ("EXT2: read block (indir 2 1) failed!\n");
+				return -1;
 			}
 			indir1_blkno =
 				__le32_to_cpu (inode->b.blocks.double_indir_block) << log2_blksz;
 		}
 
 		if (indir2_block == NULL) {
-			indir2_block = (uint32_t *) memalign(ARCH_DMA_MINALIGN,
+			indir2_block = (uint32_t *)memalign(ARCH_DMA_MINALIGN,
 							     blksz);
 			if (indir2_block == NULL) {
-				printf ("** ext2fs read block (indir 2 2) malloc failed. **\n");
-				return (-1);
+				printf ("EXT2: read block (indir 2 2) malloc failed!\n");
+				return -1;
 			}
 			indir2_size = blksz;
 			indir2_blkno = -1;
 		}
 		if (blksz != indir2_size) {
-			free (indir2_block);
+			free(indir2_block);
 			indir2_block = NULL;
 			indir2_size = 0;
 			indir2_blkno = -1;
-			indir2_block = (uint32_t *) memalign(ARCH_DMA_MINALIGN,
+			indir2_block = (uint32_t *)memalign(ARCH_DMA_MINALIGN,
 							     blksz);
 			if (indir2_block == NULL) {
-				printf ("** ext2fs read block (indir 2 2) malloc failed. **\n");
-				return (-1);
+				printf ("EXT2: read block (indir 2 2) malloc failed!\n");
+				return -1;
 			}
 			indir2_size = blksz;
 		}
-		if ((__le32_to_cpu (indir1_block[rblock / perblock]) <<
-		     log2_blksz) != indir2_blkno) {
-			status = ext2fs_devread (__le32_to_cpu(indir1_block[rblock / perblock]) << log2_blksz,
-						 0, blksz,
-						 (char *) indir2_block);
+
+		size = __le32_to_cpu(indir1_block[rblock / perblock]) << log2_blksz;
+		if (size != indir2_blkno) {
+			status = ext2fs_devread(size, 0, blksz,
+						(char *)indir2_block);
 			if (status == 0) {
-				printf ("** ext2fs read block (indir 2 2) failed. **\n");
-				return (-1);
+				printf ("EXT2: read block (indir 2 2) failed!\n");
+				return -1;
 			}
-			indir2_blkno =
-				__le32_to_cpu (indir1_block[rblock / perblock]) << log2_blksz;
+			indir2_blkno = size;
 		}
-		blknr = __le32_to_cpu (indir2_block[rblock % perblock]);
+		blknr = __le32_to_cpu(indir2_block[rblock % perblock]);
 	}
 	/* Tripple indirect.  */
 	else {
-		printf ("** ext2fs doesn't support tripple indirect blocks. **\n");
-		return (-1);
+		printf ("EXT2: doesn't support tripple indirect blocks!\n");
+		return -1;
 	}
-#ifdef DEBUG
-	printf ("ext2fs_read_block %08x\n", blknr);
-#endif
-	return (blknr);
+
+	debug("Block %08x\n", blknr);
+
+	return blknr;
 }
 
+
 static int ext2fs_read_file(struct ext2fs_node *node, int pos,
 				unsigned int len, char *buf)
 {
-- 
1.7.10

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Jorgen Lundman]

Marek Vasut wrote:
> Now I'm angry ...
>
> So, I cleaned up the ext2 filesystem code a bit. I tried to separate the changes
> to increase the reviewability.
>

If you are angry, that makes me not want to comment, especially since I 
don't belong here.

But one question that springs to mind is, the EXT4 patches by Samsung, that 
I (maybe others) are waiting for, includes the EXT2->EXT4 code merge, as 
requested by this list. This suggests that either your EXT2 changes are to 
be ignored, or that of EXT4.



-- 
Jorgen Lundman       | <lundman@lundman.net>
Unix Administrator   | +81 (0)3 -5456-2687 ext 1017 (work)
Shibuya-ku, Tokyo    | +81 (0)90-5578-8500          (cell)
Japan                | +81 (0)3 -3375-1767          (home)

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Marek Vasut]

Dear Jorgen Lundman,

> Marek Vasut wrote:
> > Now I'm angry ...
> > 
> > So, I cleaned up the ext2 filesystem code a bit. I tried to separate the
> > changes to increase the reviewability.
> 
> If you are angry, that makes me not want to comment, especially since I
> don't belong here.

Heh, sorry, didn't intend to startle you :-) Hacking on filesystems is the best 
thing to do when you're angry :-)

> But one question that springs to mind is, the EXT4 patches by Samsung, that
> I (maybe others) are waiting for, includes the EXT2->EXT4 code merge, as
> requested by this list. This suggests that either your EXT2 changes are to
> be ignored, or that of EXT4.

Right, I'd love to get the ext4 into shape eventually, but for that -- after 
this series is accepted -- I'll have to go through that patch. I already managed 
to isolate the core difference that's present between ext2 code and their ext4 
patch, but integrating it will take a bit more time as I need to study it 
properly.

Best regards,
Marek Vasut

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Jason Cooper]

Marek,

On Fri, Jun 08, 2012 at 07:31:45PM +0200, Marek Vasut wrote:
> So, I cleaned up the ext2 filesystem code a bit. I tried to separate the changes
> to increase the reviewability.
> 
> Marek Vasut (8):
>   EXT2: Indent cleanup of dev.c
>   EXT2: Indent cleanup ext2fs.c
>   EXT2: Rework ext2fs_blockgroup() function
>   EXT2: Rework ext2fs_read_file()
>   EXT2: Rework ext2fs_read_symlink()
>   EXT2: Rework ext2fs_find_file1()
>   EXT2: Rework ext2fs_iterate_dir()
>   EXT2: Rework ext2fs_read_block()
> 
>  fs/ext2/dev.c    |   74 +++---
>  fs/ext2/ext2fs.c |  775 +++++++++++++++++++++++++++---------------------------
>  2 files changed, 416 insertions(+), 433 deletions(-)

Shall I rebase my ext2load speedup patch[1] against this?  Or, wait for this
to be merged?

thx,

Jason.

[1] http://www.mail-archive.com/u-boot at lists.denx.de/msg84977.html

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Marek Vasut]

Dear Jason Cooper,

> Marek,
> 
> On Fri, Jun 08, 2012 at 07:31:45PM +0200, Marek Vasut wrote:
> > So, I cleaned up the ext2 filesystem code a bit. I tried to separate the
> > changes to increase the reviewability.
> > 
> > Marek Vasut (8):
> >   EXT2: Indent cleanup of dev.c
> >   EXT2: Indent cleanup ext2fs.c
> >   EXT2: Rework ext2fs_blockgroup() function
> >   EXT2: Rework ext2fs_read_file()
> >   EXT2: Rework ext2fs_read_symlink()
> >   EXT2: Rework ext2fs_find_file1()
> >   EXT2: Rework ext2fs_iterate_dir()
> >   EXT2: Rework ext2fs_read_block()
> >  
> >  fs/ext2/dev.c    |   74 +++---
> >  fs/ext2/ext2fs.c |  775
> >  +++++++++++++++++++++++++++--------------------------- 2 files changed,
> >  416 insertions(+), 433 deletions(-)
> 
> Shall I rebase my ext2load speedup patch[1] against this?  Or, wait for
> this to be merged?

Your patch looks great and I'm fine either way. Will these patches interfere 
with your patch too much? Can you help reviewing these please? I'd like the ext2 
code to get cleaner as it's a real mess now :-(

Wolfgang, which way do you prefer?

> thx,
> 
> Jason.
> 
> [1] http://www.mail-archive.com/u-boot at lists.denx.de/msg84977.html

Best regards,
Marek Vasut

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Marek Vasut]

Dear Wolfgang Denk,

> So, I cleaned up the ext2 filesystem code a bit. I tried to separate the
> changes to increase the reviewability.
> 
> Marek Vasut (8):
>   EXT2: Indent cleanup of dev.c
>   EXT2: Indent cleanup ext2fs.c
>   EXT2: Rework ext2fs_blockgroup() function
>   EXT2: Rework ext2fs_read_file()
>   EXT2: Rework ext2fs_read_symlink()
>   EXT2: Rework ext2fs_find_file1()
>   EXT2: Rework ext2fs_iterate_dir()
>   EXT2: Rework ext2fs_read_block()
> 
>  fs/ext2/dev.c    |   74 +++---
>  fs/ext2/ext2fs.c |  775
> +++++++++++++++++++++++++++--------------------------- 2 files changed,
> 416 insertions(+), 433 deletions(-)

I see no objections, can we apply these please?

Do you prefer pullRQ via staging or will you pick them from PW?

Best regards,
Marek Vasut

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Jason Cooper]

Marek,

On Sun, Jun 17, 2012 at 05:12:32PM +0200, Marek Vasut wrote:
> Dear Wolfgang Denk,
> 
> > So, I cleaned up the ext2 filesystem code a bit. I tried to separate the
> > changes to increase the reviewability.
> > 
> > Marek Vasut (8):
> >   EXT2: Indent cleanup of dev.c
> >   EXT2: Indent cleanup ext2fs.c
> >   EXT2: Rework ext2fs_blockgroup() function
> >   EXT2: Rework ext2fs_read_file()
> >   EXT2: Rework ext2fs_read_symlink()
> >   EXT2: Rework ext2fs_find_file1()
> >   EXT2: Rework ext2fs_iterate_dir()
> >   EXT2: Rework ext2fs_read_block()
> > 
> >  fs/ext2/dev.c    |   74 +++---
> >  fs/ext2/ext2fs.c |  775
> > +++++++++++++++++++++++++++--------------------------- 2 files changed,
> > 416 insertions(+), 433 deletions(-)
> 
> I see no objections, can we apply these please?

I apologize for the late reply on this.  It looks as though it hasn't
been pulled in yet.  My ext2load speedup patch is in Wolgang's master
tree, could you rebase this series against that and resubmit?

I'm rebuilding my dreamplug from u-boot on up and would like to test
your series while I have it torn apart.  (then, onto kernel testing).

thx,

Jason.

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Marek Vasut]

Dear Jason Cooper,

> Marek,
> 
> On Sun, Jun 17, 2012 at 05:12:32PM +0200, Marek Vasut wrote:
> > Dear Wolfgang Denk,
> > 
> > > So, I cleaned up the ext2 filesystem code a bit. I tried to separate
> > > the changes to increase the reviewability.
> > > 
> > > Marek Vasut (8):
> > >   EXT2: Indent cleanup of dev.c
> > >   EXT2: Indent cleanup ext2fs.c
> > >   EXT2: Rework ext2fs_blockgroup() function
> > >   EXT2: Rework ext2fs_read_file()
> > >   EXT2: Rework ext2fs_read_symlink()
> > >   EXT2: Rework ext2fs_find_file1()
> > >   EXT2: Rework ext2fs_iterate_dir()
> > >   EXT2: Rework ext2fs_read_block()
> > >  
> > >  fs/ext2/dev.c    |   74 +++---
> > >  fs/ext2/ext2fs.c |  775
> > > 
> > > +++++++++++++++++++++++++++--------------------------- 2 files changed,
> > > 416 insertions(+), 433 deletions(-)
> > 
> > I see no objections, can we apply these please?
> 
> I apologize for the late reply on this.  It looks as though it hasn't
> been pulled in yet.  My ext2load speedup patch is in Wolgang's master
> tree, could you rebase this series against that and resubmit?
> 
> I'm rebuilding my dreamplug from u-boot on up and would like to test
> your series while I have it torn apart.  (then, onto kernel testing).

Yes, when I get better. Or do you want to pick these, rework and resubmit?

> 
> thx,
> 
> Jason.

Best regards,
Marek Vasut

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Jason Cooper]

On Thu, Jun 28, 2012 at 04:35:24PM +0200, Marek Vasut wrote:
> Dear Jason Cooper,
> 
> > Marek,
> > 
> > On Sun, Jun 17, 2012 at 05:12:32PM +0200, Marek Vasut wrote:
> > > Dear Wolfgang Denk,
> > > 
> > > > So, I cleaned up the ext2 filesystem code a bit. I tried to separate
> > > > the changes to increase the reviewability.
> > > > 
> > > > Marek Vasut (8):
> > > >   EXT2: Indent cleanup of dev.c
> > > >   EXT2: Indent cleanup ext2fs.c
> > > >   EXT2: Rework ext2fs_blockgroup() function
> > > >   EXT2: Rework ext2fs_read_file()
> > > >   EXT2: Rework ext2fs_read_symlink()
> > > >   EXT2: Rework ext2fs_find_file1()
> > > >   EXT2: Rework ext2fs_iterate_dir()
> > > >   EXT2: Rework ext2fs_read_block()
> > > >  
> > > >  fs/ext2/dev.c    |   74 +++---
> > > >  fs/ext2/ext2fs.c |  775
> > > > 
> > > > +++++++++++++++++++++++++++--------------------------- 2 files changed,
> > > > 416 insertions(+), 433 deletions(-)
> > > 
> > > I see no objections, can we apply these please?
> > 
> > I apologize for the late reply on this.  It looks as though it hasn't
> > been pulled in yet.  My ext2load speedup patch is in Wolgang's master
> > tree, could you rebase this series against that and resubmit?
> > 
> > I'm rebuilding my dreamplug from u-boot on up and would like to test
> > your series while I have it torn apart.  (then, onto kernel testing).
> 
> Yes, when I get better. Or do you want to pick these, rework and resubmit?

Sure, no problem.  Hope you get better soon.  I'll post a v2 if there
are no glaring issues.

thx,

Jason.

[U-Boot] [PATCH 0/8] EXT2 cleanup

[Marek Vasut]

Dear Jason Cooper,

> On Thu, Jun 28, 2012 at 04:35:24PM +0200, Marek Vasut wrote:
> > Dear Jason Cooper,
> > 
> > > Marek,
> > > 
> > > On Sun, Jun 17, 2012 at 05:12:32PM +0200, Marek Vasut wrote:
> > > > Dear Wolfgang Denk,
> > > > 
> > > > > So, I cleaned up the ext2 filesystem code a bit. I tried to
> > > > > separate the changes to increase the reviewability.
> > > > > 
> > > > > Marek Vasut (8):
> > > > >   EXT2: Indent cleanup of dev.c
> > > > >   EXT2: Indent cleanup ext2fs.c
> > > > >   EXT2: Rework ext2fs_blockgroup() function
> > > > >   EXT2: Rework ext2fs_read_file()
> > > > >   EXT2: Rework ext2fs_read_symlink()
> > > > >   EXT2: Rework ext2fs_find_file1()
> > > > >   EXT2: Rework ext2fs_iterate_dir()
> > > > >   EXT2: Rework ext2fs_read_block()
> > > > >  
> > > > >  fs/ext2/dev.c    |   74 +++---
> > > > >  fs/ext2/ext2fs.c |  775
> > > > > 
> > > > > +++++++++++++++++++++++++++--------------------------- 2 files
> > > > > changed, 416 insertions(+), 433 deletions(-)
> > > > 
> > > > I see no objections, can we apply these please?
> > > 
> > > I apologize for the late reply on this.  It looks as though it hasn't
> > > been pulled in yet.  My ext2load speedup patch is in Wolgang's master
> > > tree, could you rebase this series against that and resubmit?
> > > 
> > > I'm rebuilding my dreamplug from u-boot on up and would like to test
> > > your series while I have it torn apart.  (then, onto kernel testing).
> > 
> > Yes, when I get better. Or do you want to pick these, rework and
> > resubmit?
> 
> Sure, no problem.  Hope you get better soon.  I'll post a v2 if there
> are no glaring issues.

Thanks! Keep me in CC :)

> thx,
> 
> Jason.

Best regards,
Marek Vasut

[U-Boot] [PATCH 2/8] EXT2: Indent cleanup ext2fs.c

[Jason Cooper]

On Fri, Jun 08, 2012 at 07:31:47PM +0200, Marek Vasut wrote:
> * Mostly cleanup problems reported by checkpatch.pl -f
> * Minor tweaks where it simplified the code
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  fs/ext2/ext2fs.c |  264 ++++++++++++++++++++++++++----------------------------
>  1 file changed, 128 insertions(+), 136 deletions(-)
> 
> diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
> index a4f3094..f9e9228 100644
> --- a/fs/ext2/ext2fs.c
> +++ b/fs/ext2/ext2fs.c
> @@ -27,12 +27,11 @@
>  #include <ext2fs.h>
>  #include <ext_common.h>
>  #include <malloc.h>
> -#include <asm/byteorder.h>
> +#include <linux/byteorder/generic.h>
>  
> -extern int ext2fs_devread (int sector, int byte_offset, int byte_len,
> +extern int ext2fs_devread(int sector, int byte_offset, int byte_len,
>  			   char *buf);
>  
> -

This is not applying to origin/master.  It has:

/* Magic value used to identify an ext2 filesystem.  */
#define EXT2_MAGIC              0xEF53
/* Amount of indirect blocks in an inode.  */
#define INDIRECT_BLOCKS         12
/* Maximum lenght of a pathname.  */
#define EXT2_PATH_MAX           4096
/* Maximum nesting of symlinks, used to prevent a loop.  */
#define EXT2_MAX_SYMLINKCNT     8

at line 33, 32 after.  a git blame for these lines shows only 2b918712
which is from 2004.  What is this based on?  a4f3094 isn't in my tree...

thx,

Jason.

>  struct ext2_data *ext2fs_root = NULL;
>  struct ext2fs_node *ext2fs_file;
>  int symlinknest = 0;
> @@ -65,55 +64,55 @@ static int ext2fs_blockgroup
>  
>  }
>  
> -
> -static int ext2fs_read_inode
> -	(struct ext2_data *data, int ino, struct ext2_inode *inode) {
> +static int ext2fs_read_inode(struct ext2_data *data, int ino,
> +				struct ext2_inode *inode)
> +{
>  	struct ext2_block_group blkgrp;
>  	struct ext2_sblock *sblock = &data->sblock;
> -	int inodes_per_block;
> -	int status;
> -
>  	unsigned int blkno;
>  	unsigned int blkoff;
> +	int status;
> +	int inodes_per_block;
>  
> -#ifdef DEBUG
> -	printf ("ext2fs read inode %d, inode_size %d\n", ino, inode_size);
> -#endif
> -	/* It is easier to calculate if the first inode is 0.  */
> +	debug("EXT2: read inode %d, inode_size %d\n", ino, inode_size);
> +
> +	/* It is easier to calculate if the first inode is 0. */
>  	ino--;
> -	status = ext2fs_blockgroup (data, ino / __le32_to_cpu
> -				    (sblock->inodes_per_group), &blkgrp);
> -	if (status == 0) {
> -		return (0);
> -	}
> +	status = ext2fs_blockgroup(data,
> +			ino / __le32_to_cpu(sblock->inodes_per_group),
> +			&blkgrp);
> +	if (status == 0)
> +		return 0;
>  
>  	inodes_per_block = EXT2_BLOCK_SIZE(data) / inode_size;
>  
> -	blkno = __le32_to_cpu (blkgrp.inode_table_id) +
> -		(ino % __le32_to_cpu (sblock->inodes_per_group))
> -		/ inodes_per_block;
> +	blkno = __le32_to_cpu(blkgrp.inode_table_id) +
> +			(ino % __le32_to_cpu(sblock->inodes_per_group)) /
> +			inodes_per_block;
>  	blkoff = (ino % inodes_per_block) * inode_size;
> -#ifdef DEBUG
> -	printf ("ext2fs read inode blkno %d blkoff %d\n", blkno, blkoff);
> -#endif
> -	/* Read the inode.  */
> -	status = ext2fs_devread (blkno << LOG2_EXT2_BLOCK_SIZE (data), blkoff,
> +
> +	debug("EXT2: read inode blkno %d blkoff %d\n", blkno, blkoff);
> +
> +	/* Read the inode. */
> +	status = ext2fs_devread(blkno << LOG2_EXT2_BLOCK_SIZE (data), blkoff,
>  				 sizeof (struct ext2_inode), (char *) inode);
> -	if (status == 0) {
> -		return (0);
> -	}
> +	if (status == 0)
> +		return 0;
>  
> -	return (1);
> +	return 1;
>  }
>  
> -
> -void ext2fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot)
> +static void ext2fs_free_node(struct ext2fs_node *node,
> +				struct ext2fs_node *currroot)
>  {
> -	if ((node != &ext2fs_root->diropen) && (node != currroot)) {
> -		free (node);
> -	}
> -}
> +	if (node == &ext2fs_root->diropen)
> +		return;
> +
> +	if (node == currroot)
> +		return;
>  
> +	free(node);
> +}
>  
>  static int ext2fs_read_block(struct ext2fs_node *node, int fileblock)
>  {
> @@ -490,7 +489,6 @@ static char *ext2fs_read_symlink(struct ext2fs_node *node)
>  	return (symlink);
>  }
>  
> -
>  int ext2fs_find_file1
>  	(const char *currpath, struct ext2fs_node *currroot,
>  		struct ext2fs_node **currfound, int *foundtype)
> @@ -595,172 +593,166 @@ int ext2fs_find_file1
>  	return (-1);
>  }
>  
> -
> -int ext2fs_find_file
> -	(const char *path, struct ext2fs_node *rootnode,
> -	struct ext2fs_node **foundnode, int expecttype)
> +static int ext2fs_find_file(const char *path, struct ext2fs_node *rootnode,
> +			struct ext2fs_node **foundnode, int expecttype)
>  {
>  	int status;
>  	int foundtype = FILETYPE_DIRECTORY;
>  
> -
>  	symlinknest = 0;
> -	if (!path) {
> -		return (0);
> -	}
> +	if (!path)
> +		return 0;
>  
> -	status = ext2fs_find_file1 (path, rootnode, foundnode, &foundtype);
> -	if (status == 0) {
> -		return (0);
> -	}
> -	/* Check if the node that was found was of the expected type.  */
> -	if ((expecttype == FILETYPE_REG) && (foundtype != expecttype)) {
> -		return (0);
> -	} else if ((expecttype == FILETYPE_DIRECTORY)
> -		   && (foundtype != expecttype)) {
> -		return (0);
> +	status = ext2fs_find_file1(path, rootnode, foundnode, &foundtype);
> +	if (status == 0)
> +		return 0;
> +
> +	/* Check if the node that was found was of the expected type. */
> +	if (foundtype != expecttype) {
> +		if (expecttype == FILETYPE_REG)
> +			return 0;
> +		if (expecttype == FILETYPE_DIRECTORY)
> +			return 0;
>  	}
> -	return (1);
> -}
>  
> +	return 1;
> +}
>  
> -int ext2fs_ls (const char *dirname) {
> +int ext2fs_ls(const char *dirname)
> +{
>  	struct ext2fs_node *dirnode;
>  	int status;
>  
> -	if (ext2fs_root == NULL) {
> -		return (0);
> -	}
> +	if (ext2fs_root == NULL)
> +		return 0;
>  
> -	status = ext2fs_find_file (dirname, &ext2fs_root->diropen, &dirnode,
> -				   FILETYPE_DIRECTORY);
> +	status = ext2fs_find_file(dirname, &ext2fs_root->diropen, &dirnode,
> +					FILETYPE_DIRECTORY);
>  	if (status != 1) {
> -		printf ("** Can not find directory. **\n");
> -		return (1);
> +		printf("EXT2: Can not find directory!\n");
> +		return 1;
>  	}
> -	ext2fs_iterate_dir (dirnode, NULL, NULL, NULL);
> -	ext2fs_free_node (dirnode, &ext2fs_root->diropen);
> -	return (0);
> -}
>  
> +	ext2fs_iterate_dir(dirnode, NULL, NULL, NULL);
> +	ext2fs_free_node(dirnode, &ext2fs_root->diropen);
> +
> +	return 0;
> +}
>  
> -int ext2fs_open (const char *filename) {
> +int ext2fs_open(const char *filename)
> +{
>  	struct ext2fs_node *fdiro = NULL;
>  	int status;
> -	int len;
>  
> -	if (ext2fs_root == NULL) {
> -		return (-1);
> -	}
> +	if (ext2fs_root == NULL)
> +		return -1;
> +
>  	ext2fs_file = NULL;
> -	status = ext2fs_find_file (filename, &ext2fs_root->diropen, &fdiro,
> -				   FILETYPE_REG);
> -	if (status == 0) {
> +	status = ext2fs_find_file(filename, &ext2fs_root->diropen, &fdiro,
> +					FILETYPE_REG);
> +	if (status == 0)
>  		goto fail;
> -	}
> +
>  	if (!fdiro->inode_read) {
> -		status = ext2fs_read_inode (fdiro->data, fdiro->ino,
> -					    &fdiro->inode);
> -		if (status == 0) {
> +		status = ext2fs_read_inode(fdiro->data, fdiro->ino,
> +						&fdiro->inode);
> +		if (status == 0)
>  			goto fail;
> -		}
>  	}
> -	len = __le32_to_cpu (fdiro->inode.size);
> +
>  	ext2fs_file = fdiro;
> -	return (len);
> +
> +	return __le32_to_cpu(fdiro->inode.size);
>  
>  fail:
> -	ext2fs_free_node (fdiro, &ext2fs_root->diropen);
> -	return (-1);
> +	ext2fs_free_node(fdiro, &ext2fs_root->diropen);
> +	return -1;
>  }
>  
> -
>  int ext2fs_close(void)
>  {
> -	if ((ext2fs_file != NULL) && (ext2fs_root != NULL)) {
> -		ext2fs_free_node (ext2fs_file, &ext2fs_root->diropen);
> -		ext2fs_file = NULL;
> -	}
>  	if (ext2fs_root != NULL) {
> -		free (ext2fs_root);
> +		if (ext2fs_file != NULL) {
> +			ext2fs_free_node(ext2fs_file, &ext2fs_root->diropen);
> +			ext2fs_file = NULL;
> +		}
> +
> +		free(ext2fs_root);
>  		ext2fs_root = NULL;
>  	}
> +
>  	if (indir1_block != NULL) {
> -		free (indir1_block);
> +		free(indir1_block);
>  		indir1_block = NULL;
>  		indir1_size = 0;
>  		indir1_blkno = -1;
>  	}
> +
>  	if (indir2_block != NULL) {
> -		free (indir2_block);
> +		free(indir2_block);
>  		indir2_block = NULL;
>  		indir2_size = 0;
>  		indir2_blkno = -1;
>  	}
> -	return (0);
> -}
>  
> +	return 0;
> +}
>  
> -int ext2fs_read (char *buf, unsigned len) {
> -	int status;
> -
> -	if (ext2fs_root == NULL) {
> -		return (0);
> -	}
> +int ext2fs_read(char *buf, unsigned len)
> +{
> +	if (ext2fs_root == NULL)
> +		return 0;
>  
> -	if (ext2fs_file == NULL) {
> -		return (0);
> -	}
> +	if (ext2fs_file == NULL)
> +		return 0;
>  
> -	status = ext2fs_read_file (ext2fs_file, 0, len, buf);
> -	return (status);
> +	return ext2fs_read_file(ext2fs_file, 0, len, buf);
>  }
>  
> -
> -int ext2fs_mount (unsigned part_length) {
> +int ext2fs_mount(unsigned part_length)
> +{
>  	struct ext2_data *data;
>  	int status;
>  
> -	data = malloc (sizeof (struct ext2_data));
> -	if (!data) {
> -		return (0);
> -	}
> -	/* Read the superblock.  */
> -	status = ext2fs_devread (1 * 2, 0, sizeof (struct ext2_sblock),
> -				 (char *) &data->sblock);
> -	if (status == 0) {
> +	data = malloc(sizeof(struct ext2_data));
> +	if (!data)
> +		return 0;
> +
> +	/* Read the superblock. */
> +	status = ext2fs_devread(1 * 2, 0, sizeof(struct ext2_sblock),
> +				(char *)&data->sblock);
> +
> +	if (status == 0)
>  		goto fail;
> -	}
> -	/* Make sure this is an ext2 filesystem.  */
> -	if (__le16_to_cpu (data->sblock.magic) != EXT2_MAGIC) {
> +
> +	/* Make sure this is an ext2 filesystem. */
> +	if (__le16_to_cpu(data->sblock.magic) != EXT2_MAGIC)
>  		goto fail;
> -	}
> -	if (__le32_to_cpu(data->sblock.revision_level == 0)) {
> +
> +	if (__le32_to_cpu(data->sblock.revision_level == 0))
>  		inode_size = 128;
> -	} else {
> +	else
>  		inode_size = __le16_to_cpu(data->sblock.inode_size);
> -	}
> -#ifdef DEBUG
> -	printf("EXT2 rev %d, inode_size %d\n",
> -			__le32_to_cpu(data->sblock.revision_level), inode_size);
> -#endif
> +
> +	debug("EXT2: rev %d, inode_size %d\n",
> +		__le32_to_cpu(data->sblock.revision_level), inode_size);
> +
>  	data->diropen.data = data;
>  	data->diropen.ino = 2;
>  	data->diropen.inode_read = 1;
>  	data->inode = &data->diropen.inode;
>  
> -	status = ext2fs_read_inode (data, 2, data->inode);
> -	if (status == 0) {
> +	status = ext2fs_read_inode(data, 2, data->inode);
> +	if (status == 0)
>  		goto fail;
> -	}
>  
>  	ext2fs_root = data;
>  
> -	return (1);
> -
> +	return 1;
>  fail:
> -	printf ("Failed to mount ext2 filesystem...\n");
> -	free (data);
> +	printf("EXT2: Failed to mount ext2 filesystem!\n");
> +	free(data);
>  	ext2fs_root = NULL;
> -	return (0);
> +
> +	return 0;
>  }
> -- 
> 1.7.10
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/8] EXT2: Indent cleanup ext2fs.c

[Marek Vasut]

Dear Jason Cooper,

> On Fri, Jun 08, 2012 at 07:31:47PM +0200, Marek Vasut wrote:
> > * Mostly cleanup problems reported by checkpatch.pl -f
> > * Minor tweaks where it simplified the code
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > ---
> > 
> >  fs/ext2/ext2fs.c |  264
> >  ++++++++++++++++++++++++++---------------------------- 1 file changed,
> >  128 insertions(+), 136 deletions(-)
> > 
> > diff --git a/fs/ext2/ext2fs.c b/fs/ext2/ext2fs.c
> > index a4f3094..f9e9228 100644
> > --- a/fs/ext2/ext2fs.c
> > +++ b/fs/ext2/ext2fs.c
> > @@ -27,12 +27,11 @@
> > 
> >  #include <ext2fs.h>
> >  #include <ext_common.h>
> >  #include <malloc.h>
> > 
> > -#include <asm/byteorder.h>
> > +#include <linux/byteorder/generic.h>
> > 
> > -extern int ext2fs_devread (int sector, int byte_offset, int byte_len,
> > +extern int ext2fs_devread(int sector, int byte_offset, int byte_len,
> > 
> >  			   char *buf);
> > 
> > -
> 
> This is not applying to origin/master.  It has:
> 
> /* Magic value used to identify an ext2 filesystem.  */
> #define EXT2_MAGIC              0xEF53
> /* Amount of indirect blocks in an inode.  */
> #define INDIRECT_BLOCKS         12
> /* Maximum lenght of a pathname.  */
> #define EXT2_PATH_MAX           4096
> /* Maximum nesting of symlinks, used to prevent a loop.  */
> #define EXT2_MAX_SYMLINKCNT     8
> 
> at line 33, 32 after.  a git blame for these lines shows only 2b918712
> which is from 2004.  What is this based on?  a4f3094 isn't in my tree...
> 
> thx,

Ok, I'll go through it and let you know soon. I hope about next week. Do you 
might hacking on something else in the meantime, don't waste your time here 
please until I look through it.

> Jason.
[...]

[U-Boot] [PATCH 1/8] EXT2: Indent cleanup of dev.c

[Wolfgang Denk]

Dear Marek Vasut,

In message <1339176713-13309-2-git-send-email-marex@denx.de> you wrote:
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  fs/ext2/dev.c |   74 +++++++++++++++++++++++++++------------------------------
>  1 file changed, 35 insertions(+), 39 deletions(-)
> 
> diff --git a/fs/ext2/dev.c b/fs/ext2/dev.c
> index 315ff53..f8cb8b7 100644
> --- a/fs/ext2/dev.c
> +++ b/fs/ext2/dev.c
> @@ -35,6 +35,7 @@ static disk_partition_t part_info;
>  int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
>  {
>  	ext2fs_block_dev_desc = rbdd;
> +	int ret;
>  
>  	if (part == 0) {
>  		/* disk doesn't use partition table */
> @@ -42,60 +43,53 @@ int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part)
>  		part_info.size = rbdd->lba;
>  		part_info.blksz = rbdd->blksz;
>  	} else {
> -		if (get_partition_info
> -		    (ext2fs_block_dev_desc, part, &part_info)) {
> +		ret = get_partition_info(ext2fs_block_dev_desc, part,
> +						&part_info);
> +		if (ret)

Actually this is definitely more than just an "indent" cleanup.  Please
fix the Subject:.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is not best to swap horses while crossing the river.
- Abraham Lincoln

[U-Boot] [PATCH 2/8] EXT2: Indent cleanup ext2fs.c

[Wolfgang Denk]

Dear Marek Vasut,

In message <1339176713-13309-3-git-send-email-marex@denx.de> you wrote:
> * Mostly cleanup problems reported by checkpatch.pl -f
> * Minor tweaks where it simplified the code
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  fs/ext2/ext2fs.c |  264 ++++++++++++++++++++++++++----------------------------
>  1 file changed, 128 insertions(+), 136 deletions(-)

Agai, definitely more than an "indent" cleanup.  Also, this and the
following patches don't apply any more. Please rebse and resubmit.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of COBOL cripples the mind; its teaching  should,  therefore,
be regarded as a criminal offence.
          -- Edsger W. Dijkstra, SIGPLAN Notices, Volume 17, Number 5