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

newer
[U-Boot] [PATCH] Malloc: Fix...

older
[U-Boot] [PATCH/RFC] global_data:...

Marek Vasut

8 Jun 2012 8 Jun '12

7:31 p.m.

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

Show replies by date

Marek Vasut

8 Jun 8 Jun

7:31 p.m.

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

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

Wolfgang Denk

9 Aug 9 Aug

10:01 p.m.

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

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@denx.de It is not best to swap horses while crossing the river. - Abraham Lincoln

Marek Vasut

8 Jun 8 Jun

7:31 p.m.

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

* 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

Jason Cooper

28 Jun 28 Jun

9:48 p.m.

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

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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Marek Vasut

29 Jun 29 Jun

12:03 a.m.

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

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.

[...]

Wolfgang Denk

9 Aug 9 Aug

10:02 p.m.

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

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@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

Marek Vasut

8 Jun 8 Jun

7:31 p.m.

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

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

Marek Vasut

7:31 p.m.

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

* 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

Marek Vasut

7:31 p.m.

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

* 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

Marek Vasut

7:31 p.m.

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

* 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

Marek Vasut

7:31 p.m.

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

* 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

Marek Vasut

7:31 p.m.

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

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

Jorgen Lundman

9 Jun 9 Jun

7:48 a.m.

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)

Marek Vasut

12:38 p.m.

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

Jason Cooper

7:52 p.m.

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@lists.denx.de/msg84977.html

Marek Vasut

7:55 p.m.

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@lists.denx.de/msg84977.html

Best regards, Marek Vasut

Marek Vasut

17 Jun 17 Jun

5:12 p.m.

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

Jason Cooper

28 Jun 28 Jun

3:52 p.m.

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.

Marek Vasut

4:35 p.m.

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

Jason Cooper

4:48 p.m.

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.

Marek Vasut

4:50 p.m.

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

4653

Age (days ago)

4715

Last active (days ago)

List overview

Download

21 comments

5 participants

tags (0)

participants (5)

Jason Cooper
Jorgen Lundman
Marek Vasut
Marek Vasut
Wolfgang Denk