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.
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
