On Tue, Nov 11, 2014 at 02:59:26PM +0800, Xiaoguang Wang wrote:
> When we call ext2fs_free_inode_cache(fs->icache) to free the inode
> cache, indeed it will not make fs->icache be 0, it just makes the
> local argument icache be 0, after this call, we need another
> 'fs->icache = 0' statement. So here we pass the 'ext2_filsys fs' as
> arguments directly, to make the free inode cache operation more natural.
>
> Signed-off-by: Xiaoguang Wang <wangxg.fnst@xxxxxxxxxxxxxx>
> ---
> lib/ext2fs/ext2fs.h | 2 +-
> lib/ext2fs/freefs.c | 2 +-
> lib/ext2fs/inode.c | 10 ++++++----
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
> index 506d43b..580440f 100644
> --- a/lib/ext2fs/ext2fs.h
> +++ b/lib/ext2fs/ext2fs.h
> @@ -1403,7 +1403,7 @@ extern errcode_t ext2fs_inline_data_set(ext2_filsys fs, ext2_ino_t ino,
> /* inode.c */
> extern errcode_t ext2fs_create_inode_cache(ext2_filsys fs,
> unsigned int cache_size);
> -extern void ext2fs_free_inode_cache(struct ext2_inode_cache *icache);
> +extern void ext2fs_free_inode_cache(ext2_filsys fs);
This change breaks the libext2fs ABI. If you want to change the pointer type
of the parameter, please declare a new function:
extern void ext2fs_free_inode_cache2(ext2_filsys fs);
Ideally you'd change the old function to return an error code, but it returns
void... sigh. I guess client programs are on their own.
Really, free_inode_cache is a poor interface since the ctor doesn't return a
struct ext2_inode_cache * directly, preferring to attach it to fs->icache
instead. Why are the inode cache ctor/dtor exported in ext2fs.h anyway?
Nobody seems to use them. Ted?
</rant>
Otherwise, the patch looks reasonable.
--D
> extern errcode_t ext2fs_flush_icache(ext2_filsys fs);
> extern errcode_t ext2fs_get_next_inode_full(ext2_inode_scan scan,
> ext2_ino_t *ino,
> diff --git a/lib/ext2fs/freefs.c b/lib/ext2fs/freefs.c
> index ea9742e..9c40c34 100644
> --- a/lib/ext2fs/freefs.c
> +++ b/lib/ext2fs/freefs.c
> @@ -52,7 +52,7 @@ void ext2fs_free(ext2_filsys fs)
> ext2fs_free_dblist(fs->dblist);
>
> if (fs->icache)
> - ext2fs_free_inode_cache(fs->icache);
> + ext2fs_free_inode_cache(fs);
>
> if (fs->mmp_buf)
> ext2fs_free_mem(&fs->mmp_buf);
> diff --git a/lib/ext2fs/inode.c b/lib/ext2fs/inode.c
> index 4310b82..f938c37 100644
> --- a/lib/ext2fs/inode.c
> +++ b/lib/ext2fs/inode.c
> @@ -79,10 +79,13 @@ errcode_t ext2fs_flush_icache(ext2_filsys fs)
> /*
> * Free the inode cache structure
> */
> -void ext2fs_free_inode_cache(struct ext2_inode_cache *icache)
> +void ext2fs_free_inode_cache(ext2_filsys fs)
> {
> int i;
> + struct ext2_inode_cache *icache = fs->icache;
>
> + if (!icache)
> + return;
> if (--icache->refcount)
> return;
> if (icache->buffer)
> @@ -92,7 +95,7 @@ void ext2fs_free_inode_cache(struct ext2_inode_cache *icache)
> if (icache->cache)
> ext2fs_free_mem(&icache->cache);
> icache->buffer_blk = 0;
> - ext2fs_free_mem(&icache);
> + ext2fs_free_mem(&fs->icache);
> }
>
> errcode_t ext2fs_create_inode_cache(ext2_filsys fs, unsigned int cache_size)
> @@ -131,8 +134,7 @@ errcode_t ext2fs_create_inode_cache(ext2_filsys fs, unsigned int cache_size)
> ext2fs_flush_icache(fs);
> return 0;
> errout:
> - ext2fs_free_inode_cache(fs->icache);
> - fs->icache = 0;
> + ext2fs_free_inode_cache(fs);
> return retval;
> }
>
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
