The function ext4fs_free_node() exists for dealing with "dirnode" structures that we allocate. However, we do not allocate these dynamically as needed but rather as a single instance in ext4fs_mount() that we zalloc(). Coverity scan notes that in two places we're doing what it calls a "Free of address-of expression" as we were free()'ing oldnode. However, oldnode was never directly allocated, nor any other instance which we were calling ext4fs_free_node() on. Removing this structure allows us to also restructure ext4fs_close() slightlu too.

Tested on OMAP4 Pandaboard with Fedora 23 (/boot is ext4) as well as reading and writing files from / to /boot and vice-versa and confirming they read back again correctly.

Cc: Stephen Warren swarren@nvidia.com Cc: Lukasz Majewski l.majewski@samsung.com Cc: Sjoerd Simons sjoerd.simons@collabora.co.uk Cc: Simon Glass sjg@chromium.org Cc: Przemyslaw Marczak p.marczak@samsung.com Reported-by: Coverity (CID 131278, 131091) Signed-off-by: Tom Rini trini@konsulko.com

--- I would really appreciate a few more sets of eyes on these changes. I was surprised with the conclusion I came to, but after reading the code paths a few times, I kept coming back to this same conclusion. --- fs/ext4/ext4_common.c | 38 +++++++++----------------------------- fs/ext4/ext4fs.c | 7 ------- include/ext4fs.h | 1 - 3 files changed, 9 insertions(+), 37 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c index e73223a..14096e0 100644 --- a/fs/ext4/ext4_common.c +++ b/fs/ext4/ext4_common.c @@ -1879,15 +1879,13 @@ void ext4fs_reinit_global(void) ext4fs_indir3_blkno = -1; } } + void ext4fs_close(void) { - if ((ext4fs_file != NULL) && (ext4fs_root != NULL)) { - ext4fs_free_node(ext4fs_file, &ext4fs_root->diropen); - ext4fs_file = NULL; - } if (ext4fs_root != NULL) { free(ext4fs_root); ext4fs_root = NULL; + ext4fs_file = NULL; }

ext4fs_reinit_global(); @@ -2094,10 +2092,8 @@ static int ext4fs_find_file1(const char *currpath, *(next++) = '\0'; }

- if (type != FILETYPE_DIRECTORY) { - ext4fs_free_node(currnode, currroot); + if (type != FILETYPE_DIRECTORY) return 0; - }

oldnode = currnode;

@@ -2114,26 +2110,18 @@ static int ext4fs_find_file1(const char *currpath, char *symlink;

/* Test if the symlink does not loop. */ - if (++symlinknest == 8) { - ext4fs_free_node(currnode, currroot); - ext4fs_free_node(oldnode, currroot); + if (++symlinknest == 8) return 0; - }

symlink = ext4fs_read_symlink(currnode); - ext4fs_free_node(currnode, currroot);

- if (!symlink) { - ext4fs_free_node(oldnode, currroot); + if (!symlink) return 0; - }

debug("Got symlink >%s<\n", symlink);

- if (symlink[0] == '/') { - ext4fs_free_node(oldnode, currroot); + if (symlink[0] == '/') oldnode = &ext4fs_root->diropen; - }

/* Lookup the node the symlink points to. */ status = ext4fs_find_file1(symlink, oldnode, @@ -2141,14 +2129,10 @@ static int ext4fs_find_file1(const char *currpath,

free(symlink);

- if (status == 0) { - ext4fs_free_node(oldnode, currroot); + if (status == 0) return 0; - } }

- ext4fs_free_node(oldnode, currroot); - /* Found the node! */ if (!next || *next == '\0') { *currfound = currnode; @@ -2196,22 +2180,18 @@ int ext4fs_open(const char *filename, loff_t *len) status = ext4fs_find_file(filename, &ext4fs_root->diropen, &fdiro, FILETYPE_REG); if (status == 0) - goto fail; + return -1;

if (!fdiro->inode_read) { status = ext4fs_read_inode(fdiro->data, fdiro->ino, &fdiro->inode); if (status == 0) - goto fail; + return -1; } *len = __le32_to_cpu(fdiro->inode.size); ext4fs_file = fdiro;

return 0; -fail: - ext4fs_free_node(fdiro, &ext4fs_root->diropen); - - return -1; }

int ext4fs_mount(unsigned part_length) diff --git a/fs/ext4/ext4fs.c b/fs/ext4/ext4fs.c index 258b9379..36f8023 100644 --- a/fs/ext4/ext4fs.c +++ b/fs/ext4/ext4fs.c @@ -35,12 +35,6 @@ struct ext_filesystem *get_fs(void) return &ext_fs; }

-void ext4fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot) -{ - if ((node != &ext4fs_root->diropen) && (node != currroot)) - free(node); -} - /* * Taken from openmoko-kernel mailing list: By Andy green * Optimized read file API : collects and defers contiguous sector @@ -171,7 +165,6 @@ int ext4fs_ls(const char *dirname) }

ext4fs_iterate_dir(dirnode, NULL, NULL, NULL); - ext4fs_free_node(dirnode, &ext4fs_root->diropen);

return 0; } diff --git a/include/ext4fs.h b/include/ext4fs.h index 6888adc..9cb7882 100644 --- a/include/ext4fs.h +++ b/include/ext4fs.h @@ -139,7 +139,6 @@ void ext4fs_reinit_global(void); int ext4fs_ls(const char *dirname); int ext4fs_exists(const char *filename); int ext4fs_size(const char *filename, loff_t *size); -void ext4fs_free_node(struct ext2fs_node *node, struct ext2fs_node *currroot); int ext4fs_devread(lbaint_t sector, int byte_offset, int byte_len, char *buf); void ext4fs_set_blk_dev(block_dev_desc_t *rbdd, disk_partition_t *info); long int read_allocated_block(struct ext2_inode *inode, int fileblock);