[BusyBox] please review and apply this patch.

[drwho]

 Thanks for finding yes you are correct I missed the break.

 After further reading of bb_getopt_ulflags I have seen
the light thanks for clerifying the use of the extern bb_applet_long_options I had missed that.

 Looks like now I only have to get --pidfile and --quiet working

Regards
 Sean M



>-----Original Message-----
>From: Manuel Novoa III [mailto:mjn3 at codepoet.org]
>Sent: Sunday, November 30, 2003 9:17 PM
>To: drwho
>Cc: busybox at mail.busybox.net
>Subject: Re: [BusyBox] please review and apply this patch.
>
>
>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
>
>