CODEWITHSUNDEEP
cloopsargvargc
Wickerman
Wickerman

Reputation: 417

Iterating over argv[] produces segmentation fault

I'm writing a program that reads "flags" from the command line as well as the program name. I want the program to read several flags (-i, -c and -n) and execute different functions when one or a combination of flags is called.

This is the code I started to write: 

  printf("Argv 0: %s\n", argv[0]);
  printf("Argv 1: %s\n", argv[1]);
  printf("Argv 2: %s\n", argv[2]);

  for (int i = 1; i <= argc + 1; i++) {
    if (strcmp("-i", argv[i]) == 0) {
      printf("%s\n", "found -i"); 
    }
    else{
      printf("%s\n", "did not find -i");
    }

  }

Just experimenting with one flag (-i), but I want it the read one or several flags at the same time and it to call the corresponding functions.

When I execute the program:

./program-name test -i
Argv 0: test
Argv 1: -i
Argv 2: (null)
found -i
Segmentation fault

Upvotes: 0

Views: 797

Answers (4)

KeylorSanchez
KeylorSanchez

Reputation: 1330

With your code and your loop, this line is wrong : 

(strcmp("-i", argv[i])

You should ensure that argv[i] is not NULL when you pass it to strcmp. The function is known to segfault with a NULL pointer as an argument. You should better test if argv[i] != NULL or put i <= argc in your loop instead of i <= argc + 1 which include the NULL pointer in the argv array of strings.

Upvotes: 0

daemondave
daemondave

Reputation: 305

Don't redo old code that is proven solid. It wastes time and duplicates effort. As a long time C coder, I always (when I remember) go and look at code bases for something first before I try to start from scratch. I repurpose first then create. I'm smart enough to know there were lots of smarterer coders that went before me.

Use the free and working getopt() and getopt_long() functions to parse command line arguments for you. There are lots of examples in code bases if you google search "getopt example" but here is the GNU C tutorial.

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

int
main (int argc, char **argv)
{
  int aflag = 0;
  int bflag = 0;
  char *cvalue = NULL;
  int index;
  int c;

  opterr = 0;

  while ((c = getopt (argc, argv, "abc:")) != -1)
    switch (c)
      {
      case 'a':
        aflag = 1;
        break;
      case 'b':
        bflag = 1;
        break;
      case 'c':
        cvalue = optarg;
        break;
      case '?':
        if (optopt == 'c')
          fprintf (stderr, "Option -%c requires an argument.\n", optopt);
        else if (isprint (optopt))
          fprintf (stderr, "Unknown option `-%c'.\n", optopt);
        else
          fprintf (stderr,
                   "Unknown option character `\\x%x'.\n",
                   optopt);
        return 1;
      default:
        abort ();
      }

  printf ("aflag = %d, bflag = %d, cvalue = %s\n",
          aflag, bflag, cvalue);

  for (index = optind; index < argc; index++)
    printf ("Non-option argument %s\n", argv[index]);
  return 0;
}

Upvotes: 1

Sourav Ghosh
Sourav Ghosh

Reputation: 134396

To simplify, argc is the count of command line arguments, and the arguments are held in argv[]. As C arrays have 0 based indexing, in your code, you need to change

 for (int i = 1; i <= argc + 1; i++)

to

for (int i = 1; i < argc;  i++)

to limit the access to valid argument list.

To add a bit of reference to this, quoting C11, chapter §5.1.2.2.1, Program startup (emphasis mine)

If the value of argc is greater than zero, the string pointed to by argv[0] represents the program name; argv[0][0] shall be the null character if the program name is not available from the host environment. If the value of argc is greater than one, the strings pointed to by argv[1] through argv[argc-1] represent the program parameters.

Upvotes: 3

Jonathon Reinhart
Jonathon Reinhart

Reputation: 137547

You're overrunning the bounds of argv; the condition of your for loop is incorrect. It should be i < argc.

Also, why reinvent the wheel? Look into getopt().

Upvotes: 1

Related Questions