[PATCH] add discard option -d to swapon

[Matt Whitlock]

On Monday, 24 March 2014, at 12:06 am, Tito wrote:
> On Sunday 23 March 2014 23:09:59 you wrote:
> > 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.
> 
> The problem with flags could be solved easily, take a look at how it is done in df.c:
> 
> 	enum {
> 		OPT_KILO  = (1 << 0),
> 		OPT_POSIX = (1 << 1),
> 		OPT_ALL   = (1 << 2) * ENABLE_FEATURE_DF_FANCY,
> 		OPT_INODE = (1 << 3) * ENABLE_FEATURE_DF_FANCY,
> 		OPT_BSIZE = (1 << 4) * ENABLE_FEATURE_DF_FANCY,
> 		OPT_HUMAN = (1 << (2 + 3*ENABLE_FEATURE_DF_FANCY)) * ENABLE_FEATURE_HUMAN_READABLE,
> 		OPT_MEGA  = (1 << (3 + 3*ENABLE_FEATURE_DF_FANCY)) * ENABLE_FEATURE_HUMAN_READABLE,
> 	};
> 
> that way the enum is independent from the combination of options enabled and most #ifdefs could be removed
> and we are lucky  becuase we have only 3 flags.   ;-)

That would start to get quite messy as the number of options increases:

	enum {
		OPT_a = (1 << 0),	/* all */
		OPT_d = (1 << 1) * ENABLE_FEATURE_SWAPON_DISCARD,
		OPT_f = (1 << (1 + ENABLE_FEATURE_SWAPON_DISCARD)) * ENABLE_FEATURE_SWAPON_FIXPGSZ,
		OPT_p = (1 << (1 + ENABLE_FEATURE_SWAPON_DISCARD + ENABLE_FEATURE_SWAPON_FIXPGSZ)) * ENABLE_FEATURE_SWAPON_PRI,
		OPT_s = (1 << (1 + ENABLE_FEATURE_SWAPON_DISCARD + ENABLE_FEATURE_SWAPON_FIXPGSZ + ENABLE_FEATURE_SWAPON_PRI)) * ENABLE_FEATURE_SWAPON_SUMMARY
	};

Bleh.

I prefer this approach:

	ret = getopt32(argv,
		IF_FEATURE_SWAPON_DISCARD("d::")
		IF_FEATURE_SWAPON_FIXPGSZ("f")
		IF_FEATURE_SWAPON_PRI("p:")
		IF_FEATURE_SWAPON_SUMMARY("s")
		"a"
		IF_FEATURE_SWAPON_DISCARD(, &discard)
		IF_FEATURE_SWAPON_PRI(, &prio)
	);
#if ENABLE_FEATURE_SWAPON_DISCARD
	if (ret & 1) { // -d
		/* ...handle discard option... */
	}
	ret >>= 1;
#endif
#if ENABLE_FEATURE_SWAPON_FIXPGSZ
	if (ret & 1) { // -f
		/* ...handle fixpgsz option... */
	}
	ret >>= 1;
#endif
#if ENABLE_FEATURE_SWAPON_PRI
	if (ret & 1) { // -p
		/* ...handle priority option... */
	}
	ret >>= 1;
#endif
#if ENABLE_FEATURE_SWAPON_SUMMARY
	if (ret & 1) { // -s
		/* ...handle summary option... */
	}
	ret >>= 1;
#endif
	if (ret /* & 1: redundant */) { // -a
		/* ...handle all option... */
	}

That's much cleaner and more scalable.