[BusyBox] please review and apply this patch.

Manuel Novoa III mjn3 at codepoet.org
Mon Dec 1 05:17:23 UTC 2003


Hello,

On Sun, Nov 30, 2003 at 06:39:12PM -0800,  drwho wrote:
> This is for unstable start-stop-daemon to add
> long param support IMHO very important unless you like
> rewriting all of your init scripts.

Maybe I'm tired, but there seem to be several problems with your
patch.

>  	{ "exec",		1,		NULL,		'x' },
> +	{ "quiet",		1,		NULL,		'q' },
> +	{ "verbose",	1,		NULL,		'v' },

Ok... You want to accept the -q, -v, --quiet, and --verbose options.

> -
> -	flags = bb_getopt_ulflags(argc, argv, "KSba:n:s:u:x:", 
> -			&startas, &cmdname, &signame, &userspec, &execname);
...
> +	while ((opt = getopt_long (argc, argv, "KSba:n:s:u:x:",
> +                    ssd_long_options, &option_index)) > 0)
> +    {
> +		switch (opt) {

There is absolutely no reason to bloat the code by _not_ using
bb_getopt_ulflags().  It was designed to save space by replacing
the typical 'while(getopt()) switch' code blocks.

> +			case 'x':
> +				execname = optarg;
> +				break;
> +			case 'q': /* not implemented yet */
> +			case 'v': /* not implemented yet */
> +			default:
> +				bb_show_usage();

And the new options you added simply cause the applet to show
the usage message and exit... which is exactly what they would
have done before.

I'm guessing you wanted to silently ignore the new options and
forgot a break.  But you could have accomplished the same thing
using bb_getopt_ulflags() at a minimal increase in code size.

Manuel



More information about the busybox mailing list