On Sunday, 23 March 2014, at 8:29 pm, Tito wrote:
> On Sunday 23 March 2014 00:33:04 Matt Whitlock wrote:
> > Attached are a series of three patches affecting swaponoff.
>
> Hi,
> attached you will find a alternative patch to your patches #1 and #2 that
> includes your fixes and
> improvements and reworks swap_on_off_main to allow you to implement patch #3
> without
> creating a #ifdef hell.
> I hope that the code is also easier readable than the original
> unpatched version.
So are the main improvements here the use of the IF_FEATURE* macros and testing
the ENABLE_FEATURE* macros at the C level rather than at the preprocessor
level? This is still going to create a "hell" when adding the discard flag.
Consider what will happen in the enum: if DISCARD is enabled but PRI is
disabled, then OPT_d will be (1 << 1), but if both are enabled, then OPT_p will
be (1 << 1) and OPT_d will be (1 << 2) (or vice versa). Imagine adding five
more optional features. It leads to a combinatorial explosion. It's cleaner to
downshift the bitmap after checking each bit within its appropriate #if section.
I do agree that I could clean up the construction of the options string using
the IF_FEATURE* macros. See attached patch.
>From 0181ba7e95785a85a161160e2624dcd47da7a1c4 Mon Sep 17 00:00:00 2001
From: Matt Whitlock <[email protected]>
Date: Sun, 23 Mar 2014 18:06:56 -0400
Subject: [PATCH] improve code readability using IF_FEATURE* macros
---
util-linux/swaponoff.c | 24 ++++++------------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/util-linux/swaponoff.c b/util-linux/swaponoff.c
index a7ad6db..785d57b 100644
--- a/util-linux/swaponoff.c
+++ b/util-linux/swaponoff.c
@@ -161,12 +161,8 @@ int swap_on_off_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int swap_on_off_main(int argc UNUSED_PARAM, char **argv)
{
int ret;
-#if ENABLE_FEATURE_SWAPON_DISCARD
- char *discard = NULL;
-#endif
-#if ENABLE_FEATURE_SWAPON_PRI
- unsigned prio;
-#endif
+ IF_FEATURE_SWAPON_DISCARD(char *discard = NULL;)
+ IF_FEATURE_SWAPON_PRI(unsigned prio;)
INIT_G();
@@ -178,19 +174,11 @@ int swap_on_off_main(int argc UNUSED_PARAM, char **argv)
opt_complementary = "p+";
#endif
ret = getopt32(argv, (applet_name[5] == 'n') ?
-#if ENABLE_FEATURE_SWAPON_DISCARD
- "d::"
-#endif
-#if ENABLE_FEATURE_SWAPON_PRI
- "p:"
-#endif
+ IF_FEATURE_SWAPON_DISCARD("d::")
+ IF_FEATURE_SWAPON_PRI("p:")
"a" : "a"
-#if ENABLE_FEATURE_SWAPON_DISCARD
- , &discard
-#endif
-#if ENABLE_FEATURE_SWAPON_PRI
- , &prio
-#endif
+ IF_FEATURE_SWAPON_DISCARD(, &discard)
+ IF_FEATURE_SWAPON_PRI(, &prio)
);
#endif
--
1.9.1
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
