linux-ext4 - Re: Minimal configuration for e2fsprogs

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, 20 Jun 2012 14:45:43 +1000
From:	Tony Breeds <tony@...eyournoodle.com>
To:	Andreas Dilger <adilger@...ger.ca>
Cc:	Ted Ts'o <tytso@....edu>, linux-ext4@...r.kernel.org
Subject: Re: Minimal configuration for e2fsprogs

On Tue, Jun 19, 2012 at 12:01:54AM -0600, Andreas Dilger wrote:

Thanks for the feedback.

> That is what I would do, though perhaps with macro names that are not
> so confusingly similar to the actual macro names, which might otherwise
> cause confusion if someone doesn't read the code very closely.  Like:
> 
> #ifdef ENABLE_MMP
> ...
> #define EXT2_LIB_INCOMPAT_SUPP_MMP EXT4_FEATURE_INCOMPAT_MMP
> #else
> #define EXT2_LIB_INCOMPAT_SUPP_MMP (0)
> #endif

Okay, since Ted doesn't mind I went with:
#ifdef CONFIG_MMP
#define EXT4_LIB_INCOMPAT_MMP          EXT2_FEATURE_INCOMPAT_MMP
#else
#define EXT4_LIB_INCOMPAT_MMP          (0)
#endif

Of course we /could/ #undef them once we're done to be really clear
that they're special.


> This should at least print a message like "This version of debugfs
> does not support MMP.  Recompile with --enable-mmp if necessary."

Okay done.

> Having conditionals inline in the code is frowned upon.  Better to
> put the conditionals inside ext2fs_mmp_stop(), or declare a no-op
> inline function and keep the rest of the code clean.

Okay I went with putting the conditionals in the ext2fs_mmp_*()
functions.  As this removes the changes to ext2fs.h and
lib/ext2fs/Makefile.in
 
> These should all conditionally become static inline no-op functions
> here (returning zero as needed) so that the calling code does not need
> messy #ifdefs everywhere.

Well here you and Ted disagree :)  I'm happy with either.  For the time
being I've gone with Ted's idea.  I'll let you guys decide on the
relative merits of each approach and then go with whichever "wins"

I'll include by v3 patch in my reply to Ted.

Yours Tony

Content of type "application/pgp-signature" skipped