From: Tony Breeds <tony@bakeyournoodle.com>
To: Andreas Dilger <adilger@dilger.ca>
Cc: Ted Ts'o <tytso@mit.edu>, linux-ext4@vger.kernel.org
Subject: Re: Minimal configuration for e2fsprogs
Date: Wed, 20 Jun 2012 14:45:43 +1000 [thread overview]
Message-ID: <20120620044543.GA11330@thor.bakeyournoodle.com> (raw)
In-Reply-To: <86585638-21F5-4FEE-9313-28AC122FCCEE@dilger.ca>
[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]
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
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2012-06-20 4:45 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-15 4:24 Minimal configuration for e2fsprogs Tony Breeds
2012-06-16 0:08 ` Ted Ts'o
2012-06-18 5:58 ` Tony Breeds
2012-06-18 17:12 ` Ted Ts'o
2012-06-19 5:48 ` Tony Breeds
2012-06-19 6:01 ` Andreas Dilger
2012-06-19 13:56 ` Ted Ts'o
2012-06-20 5:26 ` Tony Breeds
2012-06-20 5:33 ` Tony Breeds
2012-06-20 14:14 ` Andreas Dilger
2012-06-20 4:45 ` Tony Breeds [this message]
2012-06-26 2:10 ` Tony Breeds
2012-06-26 2:33 ` Theodore Ts'o
2012-06-26 2:47 ` Tony Breeds
2012-06-27 11:21 ` Tony Breeds
2012-06-27 12:54 ` Theodore Ts'o
2012-06-28 2:43 ` Tony Breeds
2012-07-30 21:45 ` Theodore Ts'o
2012-07-31 5:21 ` Tony Breeds
2012-07-31 19:57 ` Theodore Ts'o
2012-08-01 5:42 ` Tony Breeds
2012-07-30 21:47 ` [PATCH 1/7] e2fsck: add SIGABRT to list of signals processed by sigcatcher Theodore Ts'o
2012-07-30 21:47 ` [PATCH 2/7] libext2fs: use abort() instead of perror()/exit() Theodore Ts'o
2012-07-31 18:34 ` Andreas Dilger
2012-07-31 20:04 ` Theodore Ts'o
2012-07-30 21:47 ` [PATCH 3/7] libext2fs: use strcpy()/strcat() instead of sprintf() in bmap functions Theodore Ts'o
2012-07-30 21:47 ` [PATCH 4/7] libext2fs: move ext2fs_get_num_dirs to its own file Theodore Ts'o
2012-07-30 21:47 ` [PATCH 5/7] libext2fs: call numeric_progress functions through a operations struct Theodore Ts'o
2012-07-30 21:47 ` [PATCH 6/7] libext2fs: remove debugging printf from ext2fs_group_desc_csum Theodore Ts'o
2012-07-30 21:47 ` [PATCH 7/7] libext2fs: enforce the block group descriptor size in ext2fs_open() Theodore Ts'o
2012-07-31 18:38 ` Andreas Dilger
2012-07-31 20:09 ` Theodore Ts'o
2012-08-01 20:45 ` Andreas Dilger
2012-08-03 0:00 ` Theodore Ts'o
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120620044543.GA11330@thor.bakeyournoodle.com \
--to=tony@bakeyournoodle.com \
--cc=adilger@dilger.ca \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.