linux-ext4 - Re: [PATCH 1/4] e2fsprogs/libext2fs: replace ext2fs_free_inode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 12 Nov 2014 17:38:46 +0800
From:	Xiaoguang Wang <wangxg.fnst@...fujitsu.com>
To:	"Theodore Ts'o" <tytso@....edu>
CC:	"Darrick J. Wong" <darrick.wong@...cle.com>,
	<linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 1/4] e2fsprogs/libext2fs: replace ext2fs_free_inode_cache()
 argument

Hi,

On 11/12/2014 12:12 AM, Theodore Ts'o wrote:
> On Tue, Nov 11, 2014 at 12:14:51AM -0800, Darrick J. Wong wrote:
>> 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?
> 
> Looking at the history, the change happened in commit 603e5ebc, which
> changed the internal representation of the ext2_inode_cache.  At that
> point, I moved the inode cache freeing function from
> lib/ext2fs/freefs.c to lib/ext2fs/inode.c, on the theory that it made
> more sense to keep all of the code that handled the representation of
> the inode cache was kept in the same place.
> 
> I agree I should have declared the function in ext2fsP.h instead of
> ext2fs.h; the interface was not great, but that was probably because
> it was originally intended as an internal interface, and I didn't
> bother to fix it up before moving it function to another .c file.
> 
> At this point, we have a couple of choices.  
> 
> (1) define a new interface with a new name (ext2fs_free_inode_cache2)
> 
> (2) move the function back to freefs.c and make it be static, so that
> the function signature disappears from the shared library.
> 
> (3) ignore the problem because it's highly unlikely anyone outside of
> libext2fs will need to use it, and improving the API/ABI doesn't
> really matter all that much.  Instead, just change the callers to
> clear fs->icache after calling ext2fs_free_inode_cache().
> 
> My preference is (2) or (3).

OK, I'll choose (3) :) thanks!
Version 2 will be sent soon.

Regards,
Xiaoguang Wang
> 
> Cheers,
> 
> 						- Ted
> .
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html