>-----Original Message-----
>From: Joe Eykholt [mailto:[email protected]]
>Sent: Friday, June 26, 2009 10:20 AM
>To: Ma, Steve
>Cc: [email protected]
>Subject: Re: [Open-FCoE] [PATCH] fcoe-utils: fcoeadm -i and fcoeadm --
>interface not consistent
>
>
>
>Steve Ma wrote:
>> fcoeadm --interface  without providing any additional arguments
>> results in an error:
>>         ./fcoeadm: option '--interface' requires an argument
>> However, fcoeadm -i displays all the interfaces correctly.
>>
>> There are two problems:
>> 1) --interface option is not specified to have optional argument,
>>    in {"interface", 1, 0, 'i'} of fcoeadm_options.
>> 2) The code to parse the --interface option under case 'i' are
>>    incorrect.
>>
>> This patch is to fix these two problems by replaing
>> {"interface", 1, 0, 'i'} by {"interface", 2, 0, 'i'} in fcoeadm_options
>> to allow --interface to have optional arguments. Secondly,
>> check the return pointer of s in "case 'i' in case it is NULL.
>>
>>
>>
>> Signed-off-by: Steve Ma <[email protected]>
>> ---
>>
>>  fcoeadm.c |   10 ++++++----
>>  1 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/fcoeadm.c b/fcoeadm.c
>> index 6b63e17..2e44b34 100644
>> --- a/fcoeadm.c
>> +++ b/fcoeadm.c
>> @@ -43,7 +43,7 @@ static struct option fcoeadm_opts[] = {
>>      {"create", 1, 0, 'c'},
>>      {"destroy", 1, 0, 'd'},
>>      {"reset", 1, 0, 'r'},
>> -    {"interface", 1, 0, 'i'},
>> +    {"interface", 2, 0, 'i'},
>>      {"target", 2, 0, 't'},
>>      {"lun", 2, 0, 'l'},
>>      {"stats", 1, 0, 's'},
>> @@ -477,9 +477,11 @@ int main(int argc, char *argv[])
>>                              goto error;
>>                      s = NULL;
>>                      if (argc == 2) {
>> -                            if (argv[1][1] == '-')
>> -                                    s = strchr(argv[1], '=')+1;
>> -                            else
>> +                            if (argv[1][1] == '-') {
>> +                                    s = strchr(argv[1], '=');
>> +                                    if (s)
>> +                                            s += 1;
>> +                            } else
>>                                      s = argv[1]+2;
>>                      } else
>>                              s = argv[2];
>
>Can't you just set s = optarg (declared extern char *optarg) instead?
>See man 3 getopt.  All of this parsing should be done by getopt for you.
>
>       Joe

I looked into the man page, and tried it. It does that for me.
I will resubmit with a new patch. -Steve
_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel

Reply via email to