0

I am building this pizza program using command-line arguments and interface. It should return the ingredients from the arguments.

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

int main(int argc, char *argv[])
{
    char *delivery = "";
    int thick = 0;
    int count = 0;
    char ch;


    while(ch = getopt(argc, argv, "d:t") != EOF)
        switch(ch)
        {
            case 'd' :
            delivery = optarg;
            break;

            case 't' :
            thick = 1;
            break;

            default:
            fprintf(stderr, "Invalid option : %s", optarg);
            return 1;
        }

    argc -= optind;
    argv += optind;

    if(thick)
        puts("thick crust");

    if(delivery[0])
        printf("To be delivered %s", delivery);

    puts("ingredients :");

    for(count = 0; count<argc; count++)
    {
        puts(argv[count]);
    }

    return 0;
}

When running this program using command prompt in windows as :

program -d now -t

it was returning an error :

invalid option: now

How should I run this program and why is this error showing?

Garf365
  • 3,619
  • 5
  • 29
  • 41
Sudeep
  • 45
  • 6
  • why are you doing `argc -=`/`argv +=`? changing loop sentinels inside the loop is usually a bad idea. – Marc B Jun 22 '16 at 14:35
  • I don't think it's inside the loop. – Sudeep Jun 22 '16 at 14:40
  • Off topic : always put bracket (`{..}`) for control statements [see here](http://stackoverflow.com/questions/2125066/is-it-bad-practice-to-use-an-if-statement-without-brackets). And take care about your indentation => we can think `argc -= optind;` and other lines are in the loop because they are indented like `switch` statement which is in loop – Garf365 Jun 22 '16 at 14:41
  • ah yes. the indentation makes it look like it is. there's no `{}` on the while. – Marc B Jun 22 '16 at 14:42

1 Answers1

4

You've made five mistakes (two important, three less so) and one of the important mistakes is masking the other.

while(ch = getopt(argc, argv, "d:t") != EOF)

Important mistake #1: The operator precedence of = is lower than !=, so this assigns the result of the comparison to ch, not the return value of getopt as you expected. Less important mistake #1: getopt returns −1 when it reaches the end of the options. EOF is not necessarily equal to −1.

You should have written

while ((ch = getopt(argc, argv, "d:t")) != -1)

Now, when invoked as you described, in the first iteration the value returned from getopt will be 'd', which is not equal to EOF (nor to −1), so the value assigned to ch will be the number 1 (also known as Control-A, or U+0001 START OF HEADING). This number is not equal to either 'd' or 't' (the C standard guarantees 0 < 'a' < 'b' < 'c' < 'd' < ... < 'z', so even if we don't assume ASCII, 1 could only ever be equal to 'a') so the default branch of the switch is taken, and we do this:

fprintf(stderr, "Invalid option : %s", optarg);
return 1;

And here is important mistake 2: you should print ch, not optarg, when you get an invalid option. ch is the option; optarg is the argument to the option, if any. If you had printed ch you would have realized that the value in ch was not what you expected it to be.

The other less important mistakes are

char ch;

This should be int ch; Like in a getchar loop, the value returned from getopt when it reaches the end of the arguments is outside the range of char.

fprintf(stderr, "Invalid option : %s", optarg);
printf("To be delivered %s", delivery);

Both of these need a \n at the end of the string to be printed.

In the original form of your question, your code was badly indented, but I suspect that was because you used 4 spaces for a single level of indentation and an 8-space-wide hard TAB for a second level; it is a long-standing bug in the Stack Overflow interface that this kind of code gets mangled when pasted into a question. So it's not your fault. (You should indent with spaces only, though.) The more serious style problem with your code is that some of your single-statement loops have curly braces around them and some of them don't. Whether or not one should put curly braces around single-statement blocks in C is one of the oldest holy wars; I personally think both sides are wrong, but never mind that; the important thing you should learn is, pick one style or the other and stick to it consistently throughout your code.

zwol
  • 135,547
  • 38
  • 252
  • 361