0

here is my code

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

int main(int argc,char* argv[])
{
    int a;
    for(int i=1;i<=argc;i++){
        a+=atoi(argv[i]);
    }
    printf ("%d",a);
}

I keep getting segmentation faults but i am trying to add up all elements of the command line so for example ./a.out 5 6 7 would give 18 as the output, cheers.

mch
  • 9,424
  • 2
  • 28
  • 42
  • 3
    arrays are 0 based (first element in index 0). you need: `for(int i=0;i – wohlstad May 05 '22 at 11:21
  • @wohlstad that seems like a solution to the problem, shouldn't that be an answer then? – CompuChip May 05 '22 at 11:22
  • 2
    @wohlstad Not really, `argv[0]` is the program name in one form or another, so it shouldn't be used. – Some programmer dude May 05 '22 at 11:24
  • @Someprogrammerdude thanks for the correction. You are right of course. I just focused on the issue of accessing array elements in general. – wohlstad May 05 '22 at 11:27
  • 3
    On an unrelated note: You don't initialize the variable `a`. It will have an *indeterminate* (consider it garbage) value. – Some programmer dude May 05 '22 at 11:27
  • On yet another (unrelated) note: The `atoi` function doesn't really have any kind of validation of the strings you pass to it. There's no way to differ between invalid input and the input `"0"` for example. If you want proper validation use [`strtol`](https://en.cppreference.com/w/c/string/byte/strtol) instead. – Some programmer dude May 05 '22 at 11:30

2 Answers2

4

The problem (with the crash) is the loop itself:

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

The argc argument is the number of arguments passed to the program, including the "program name" at argv[0]. So valid indexes for the actual arguments are argv[1] to argv[argc - 1].

Furthermore the argv array is terminated by a null pointer, which will be at argv[argc].

Since you include argv[argc] in your loop you pass a null pointer to atoi which leads to undefined behavior and likely crashes.

The simple solution is to use less-than < instead of less-than-or-equal as the loop condition:

for(int i=1;i<argc;i++)
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
2

You never initialized a to 0. Also, use strtol() function.

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

int main(int argc, char **argv) {
    int a = 0;
    for (int i = 1; i < argc; i++) {
        a += strtol(argv[i], NULL, 10);
    }
    printf("%d\n", a);
    return EXIT_SUCCESS;
}
Darth-CodeX
  • 2,166
  • 1
  • 6
  • 23