Search code examples
cgetopt

getopt() doesn't return the next argument


Take a look at this code:-

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char *argv[]){
    char ch;
    int value;
    while((ch = getopt(argc, argv, "n: o"))!=EOF){
        switch(ch){
            case 'n':
                value = atoi(optarg);
                fprintf(stdout,"\nParameter n");
                //Do something
                break;
            case 'o':
                fprintf(stdout,"\nParameter 0");
                //Do something
                break;
            default:
                fprintf(stdout,"\nInvalid!");
        }
        argc -= optind;
        argv += optind;
    }
}

When I pass following arguments

./program -n 123 -o

I get this result

Parameter n

while I expect to get this

Parameter n
Parameter o

why isn't getopt() returning next argument in 2nd iteration of the loop?

Update

So the code should be like this:-

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char *argv[]){
    char ch;
    int value;
    while((ch = getopt(argc, argv, "n:o"))!=-1){
        switch(ch){
            case 'n':
                value = atoi(optarg);
                fprintf(stdout,"Parameter n (%d)\n", value);
                //Do something
                break;
            case 'o':
                fprintf(stdout,"Parameter o\n");
                //Do something
                break;
            default:
                fprintf(stdout,"Invalid!\n");
                break;
        }
    }
}

Solution

  • Main problem

    Inside the loop, you have:

        argc -= optind;
        argv += optind;
    

    That's a disaster — don't do it. You might use those statements after the loop is complete (once), but not in the body of the loop. It forces the code to skip options; in severe cases, it can end up trying to parse a null pointer or the environment, neither of which is likely to help (and both are undefined behaviour, so YMMV).

    Tangential issues

    Note that you've specified that a blank is one of the options. Thus someone could write:

    ./a.out -' ' -n 123 -o
    

    and the blank would be treated as a flag option (like -o). It probably isn't what you had in mind. Use "n:o" without a space in it.

    You print:

    fprintf(stdout,"\nParameter 0");
    

    Three minor issues with that statement:

    1. The 0 should be o — they're different.
    2. Put newlines at the end of output formats rather than at the beginning (unless you want double-spacing). Note that output may not be generated until a newline is printed, so the newline at the end ensures that printed data appears more timely.
    3. It is aconventional to use fprintf(stdout, …) rather than printf(…). Not precisely wrong, but unusual.

    The getopt() function is defined by POSIX to return -1 and not EOF when it finishes the option processing. This is so that its declaration in <unistd.h> isn't tied to the definition of EOF from <stdio.h>. (The Rationale section says explicitly: The getopt() function shall return -1, rather than EOF, so that <stdio.h> is not required.) Some systems, historically, declared getopt() in <stdio.h>, but POSIX places it in <unistd.h> and says that it returns -1.

    You should include a break; after the default: case in the switch. It's a basic defensive programming measure — it ensures no fall-through even if someone adds another case label after the default: label.