Skip to contentSkip to conversation

EXT2 cleanup

PATCH New Marek Vasut 5 participants 22 messages Jun 8, 2012 – Aug 9, 2012 ext2 file system

lore
Share & tools
lore

Marek Vasut

author
#0 lore raw

cover EXT2 cleanup

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(-)
Signature · 1 line
1.7.10
author
#1 lore raw

PATCH 1/8 EXT2: Indent cleanup of dev.c

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/dev.c |   74 +++++++++++++++++++++++++++------------------------------
 1 file changed, 35 insertions(+), 39 deletions(-)
fs/ext2/dev.c +35 -39
index 315ff53..f8cb8b7 100644
@@ -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;
 }
Signature · 1 line
1.7.10
#2 lore raw

PATCH 1/8 EXT2: Indent cleanup of dev.c

Dear Marek Vasut,

Quoted 19 lines · In message <[email protected]> you wrote:
Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/dev.c |   74 +++++++++++++++++++++++++++------------------------------
 1 file changed, 35 insertions(+), 39 deletions(-)
fs/ext2/dev.c +1 -0
index 315ff53..f8cb8b7 100644
@@ -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 */

fs/ext2/dev.c int ext2fs_set_blk_dev(block_dev_desc_t *rbdd, int part) · line 48

Quoted 8 lines
fs/ext2/dev.c +3 -2
@@ -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

Signature · 5 lines
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
author
#3 lore raw

PATCH 2/8 EXT2: Indent cleanup ext2fs.c

In reply to Marek Vasut

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

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |  264 ++++++++++++++++++++++++++----------------------------
 1 file changed, 128 insertions(+), 136 deletions(-)
fs/ext2/ext2fs.c +128 -136
index a4f3094..f9e9228 100644
@@ -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;
 }
Signature · 1 line
1.7.10
#4 lore raw

PATCH 2/8 EXT2: Indent cleanup ext2fs.c

Quoted 15 lines · Marek Vasut

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

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |  264 ++++++++++++++++++++++++++----------------------------
 1 file changed, 128 insertions(+), 136 deletions(-)
fs/ext2/ext2fs.c +0 -0
index a4f3094..f9e9228 100644
@@ -27,12 +27,11 @@
 #include <ext2fs.h>

fs/ext2/ext2fs.c line 35

Quoted 10 lines
fs/ext2/ext2fs.c +2 -3
 #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.

Quoted 373 lines
fs/ext2/ext2fs.c +126 -133
 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;
 }
Signature · 6 lines
1.7.10

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
author
#5 lore raw

PATCH 2/8 EXT2: Indent cleanup ext2fs.c

Dear Jason Cooper,

Quoted 43 lines · Marek Vasut
Quoted 20 lines

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

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>

---

 fs/ext2/ext2fs.c |  264

++++++++++++++++++++++++++---------------------------- 1 file changed, 128 insertions(+), 136 deletions(-)

fs/ext2/ext2fs.c +0 -0
index a4f3094..f9e9228 100644
@@ -27,12 +27,11 @@
 
 #include <ext2fs.h>
 #include <ext_common.h>
 #include <malloc.h>

fs/ext2/ext2fs.c line 38

Quoted 10 lines
fs/ext2/ext2fs.c +2 -3
 
-#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

Quoted 4 lines

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.

Quoted 1 line

Jason.

[...]

#6 lore raw

PATCH 2/8 EXT2: Indent cleanup ext2fs.c

In reply to Marek Vasut

Dear Marek Vasut,

Quoted 8 lines · In message <[email protected]> you wrote:

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

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 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

Signature · 6 lines
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
author
#7 lore raw

PATCH 3/8 EXT2: Rework ext2fs_blockgroup() function

In reply to Marek Vasut

Rework the function, constify every possible component.

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |   31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)
fs/ext2/ext2fs.c +14 -17
index f9e9228..69fe1d1 100644
@@ -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,
Signature · 1 line
1.7.10
author
#8 lore raw

PATCH 4/8 EXT2: Rework ext2fs_read_file()

In reply to Marek Vasut

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

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |   65 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 31 deletions(-)
fs/ext2/ext2fs.c +34 -31
index 69fe1d1..0d54ae6 100644
@@ -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,
Signature · 1 line
1.7.10
author
#9 lore raw

PATCH 5/8 EXT2: Rework ext2fs_read_symlink()

In reply to Marek Vasut

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

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |   45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)
fs/ext2/ext2fs.c +23 -22
index 0d54ae6..22cc9c2 100644
@@ -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
Signature · 1 line
1.7.10
author
#10 lore raw

PATCH 6/8 EXT2: Rework ext2fs_find_file1()

In reply to Marek Vasut

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

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |  101 +++++++++++++++++++++++++++---------------------------
 1 file changed, 50 insertions(+), 51 deletions(-)
fs/ext2/ext2fs.c +50 -51
index 22cc9c2..10745f6 100644
@@ -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,
Signature · 1 line
1.7.10
author
#11 lore raw

PATCH 7/8 EXT2: Rework ext2fs_iterate_dir()

In reply to 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 <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |  139 +++++++++++++++++++++++++++---------------------------
 1 file changed, 69 insertions(+), 70 deletions(-)
fs/ext2/ext2fs.c +69 -70
index 10745f6..d55c8c5 100644
@@ -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)
Signature · 1 line
1.7.10
author
#12 lore raw

PATCH 8/8 EXT2: Rework ext2fs_read_block()

In reply to Marek Vasut

Fix indentation complains from checkpatch.pl

Signed-off-by Marek Vasut <[email protected]>
Cc Wolfgang Denk <[email protected]>
---
 fs/ext2/ext2fs.c |  130 ++++++++++++++++++++++++++----------------------------
 1 file changed, 63 insertions(+), 67 deletions(-)
fs/ext2/ext2fs.c +63 -67
index d55c8c5..5bb8851 100644
@@ -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)
 {
Signature · 1 line
1.7.10
#13 lore raw

cover EXT2 cleanup

In reply to Marek Vasut
Quoted 5 lines · 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.

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.

Signature · 4 lines
Jorgen Lundman       | <[email protected]>
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)
#14 lore raw

cover EXT2 cleanup

Dear Jorgen Lundman,

Quoted 8 lines
Quoted 4 lines · 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.

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 :-)

Quoted 4 lines

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

#15 lore raw

cover EXT2 cleanup

In reply to Marek Vasut

Marek,

Quoted 13 lines · Marek Vasut

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()

Quoted 3 lines
 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

author
#16 lore raw

cover EXT2 cleanup

Dear Jason Cooper,

Quoted 16 lines

Marek,

Quoted 13 lines · Marek Vasut

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()

Quoted 7 lines
Quoted 4 lines
 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?

Quoted 5 lines

thx,

Jason.

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

Best regards,
Marek Vasut

author
#17 lore raw

cover EXT2 cleanup

In reply to Marek Vasut

Dear Wolfgang Denk,

Quoted 13 lines

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()

Quoted 4 lines
 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

#18 lore raw

cover EXT2 cleanup

Marek,

Quoted 15 lines · Marek Vasut

Dear Wolfgang Denk,

Quoted 13 lines

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()

Quoted 6 lines
Quoted 4 lines
 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.

author
#19 lore raw

cover EXT2 cleanup

Dear Jason Cooper,

Quoted 24 lines

Marek,

Quoted 21 lines · Marek Vasut

Dear Wolfgang Denk,

Quoted 18 lines

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(-)

Quoted 8 lines
Quoted 1 line

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?

Quoted 4 lines

thx,

Jason.

Best regards,
Marek Vasut

#20 lore raw

cover EXT2 cleanup

Quoted 28 lines · Marek Vasut

Dear Jason Cooper,

Quoted 26 lines

Marek,

Quoted 22 lines · Marek Vasut

Dear Wolfgang Denk,

Quoted 13 lines

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()

Quoted 5 lines
 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?

Quoted 8 lines
Quoted 6 lines

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.

author
#21 lore raw

cover EXT2 cleanup

Dear Jason Cooper,

Quoted 33 lines · Marek Vasut
Quoted 32 lines

Dear Jason Cooper,

Quoted 30 lines

Marek,

Quoted 15 lines · Marek Vasut

Dear Wolfgang Denk,

Quoted 13 lines

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()

Quoted 7 lines
Quoted 5 lines
 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?

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

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 :)

Quoted 3 lines

thx,

Jason.

Best regards,
Marek Vasut