How to sum all command line arguments in C?

Question

I have an assignment. The program is to print the sum of all command line arguments in C. I tried this code it compiles but throws an error after passed arguments in the console. Below is the code.

/* Printing sum of all command line arguments */
#include <stdio.h>

int main(int argc, char *argv[]) {
    int sum = 0, counter;

    for (counter = 1; counter <= argc; counter++) {
       sum = atoi(sum) + atoi(argv[counter]);
    }
    printf("Sum of %d command line arguments is: %d\n", argc, sum);
}

After compiling it outputs a Segmentation fault (core dumped) error. Your experience may solve my problem.

Below is my edited code:

/* Printing sum of all command line arguments*/
#include <stdio.h>
#include <stdlib.h> // Added this library file

int main (int argc, char *argv[]) {
    int sum = 0, counter;

    for (counter = 1; counter < argc; counter++) {
        // Changed the arithmetic condition
        sum = sum + atoi(argv[counter]);
        // Removed the atoi from sum variable
    }
    printf("Sum of %d command line arguments is: %d\n", argc, sum);
}

Answer 1

Because you are iterating until counter == argc, you are passign a NULL pointer to atoi(), It's simple, just rely on the fact that the argv array has a NULLsentinel, and do this

/* Printing sum of all command line arguments*/
#include <stdlib.h> /* For `atoi()' */
#include <stdio.h>  /* For `printf()' */

int main(int argc, char *argv[])
{
    int sum;
    sum = 0;
    for (int counter = 1; argv[counter] != NULL; ++counter)     {
        sum += atoi(argv[counter]);
    }
    printf("Sum of %d command line arguments is: %d\n", argc, sum);
}

Note that atoi(sum) is undefined behavior because sum is an int and is not a valid pointer. While atoi() will try to dereference it. Also aoti() stands for ascii to integer and sum is already an integer so the conversion doesn't make sense.

Finally, include stdlib.h for atoi(). I know you didn't include it because I have warnings enabled on my compiler and it warned me that atoi() was implicitly defined. It might work, but just because undefined behavior is that, UNDEFINED.

Also, note that there is no way to know whether the passed argument is an integer, because atoi() cannot perform error checking. You might want to use strtol() instead and check if all the values are integers.

So ... this is how you would write a more robust version of this program

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

int main(int argc, char *argv[])
{
    int sum;
    sum = 0;
    for (int counter = 1; argv[counter] != NULL; ++counter)     {
        char *endptr;
        sum += strtol(argv[counter], &endptr, 10);
        if (*endptr != '\0') {
            fprintf(stderr, "error: the `%d-th' argument `%s', is not a valid integer\n", counter, argv[counter]);
            return EXIT_FAILURE;
        }
    }
    printf("sum of %d command line arguments is: %d\n", argc, sum);
    return EXIT_SUCCESS;
}

EDIT: To address this comment

There is a possibility that argc == 0, for instance if you execute the program through one of the exec*() functions. In that case you must check before starting the loop or argv[counter] will be one element after the last, i.e. out of bounds.

Answer 2

It is specified that argv[argc] will always be a null pointer. You loop once too many, and pass this null pointer to atoi, leading to undefined behavior.

Change your loop conditions to be counter < argc.

And sum already is an integer, you do not need to convert it to an integer with atoi. That atoi(sum) will also lead to undefined behavior as the very first iteration will pass zero to atoi, which could be seen as a null pointer as well.

Answer 3

The last element of argv is defined to be NULL, and the first one is always the program name. Therefore you can reduce your code to

#include "stdio.h"

int main(int argc, char *argv[])
{
    int sum = 0;
    for (int i = 1; argv[i]; ++i){
        sum += atoi(argv[i]);
    }
    printf("Sum of %d command line arguments is: %d\n", argc, sum);
}

In your code, the behaviour of atoi(sum) and what boils down to argv[argc] on the final iteration will be undefined.

Answer 4

A recursive version, just for the heck of it :)

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

int main(int argc, char **argv)
{   
    static int sum;

    if (*++argv == NULL)
        return !printf("sum: %d argc %d\n", sum, argc - 1);

    sum += atoi(*argv);
    return main(argc, argv);
}

Answer 5

/* My example. Should work, though untested. */
#include <stdio.h>

int main(int argc, char *argv[])
{
    int sum, index;    // Generally considered better form to put them on separate lines.
    sum = 0;

    if(argc > 1) {    
        for(index = 1; index < argc; index++) {
            sum += atoi(argv[index]);
        }
        printf("%d Command-line args\nSum = %d\n", argc, sum);
    } 
    else {
        printf("Not enough command-line args\n");
    }
}

Make sure to try you hardest to adhere to a specific formatting style for your code. Search for style guides if you want to define yours by name. GNU C is a good starting point. They'll make your code more readable for you, easier to debug, and help others understand what's going on in it.

Couple of corrections for your aforementioned code..

/* Your code with corrections */
#include "stdio.h"

int main(int argc, char *argv[])
{
   int sum=0,counter; 
   // 1. Normally you want to give each variable it's own line. Readability is important.
   // 2. Don't initialize on declaration either. Refer to my example.
   // 3. Always add spaces between operators and at the end of statements for better readability.
    for(counter=1;counter<=argc;counter++)
    {
       // 4. atoi(sum) is unneccessary as it interprets your int sum as a const char*, 
       // which as far as I can tell should just give you back it's ascii value. 
       // It might be undefined however, so I would remove it regardless.
       // 5. Your segfault issue is because you iterate over the end of the array.
       // You try to access the [argc] value of the array (remember 0-based
       // indexing means a size val is always 1 greater than the greatest index).
       // Make 'counter<=argc' into 'counter < argc'

       sum = atoi(sum) + atoi(argv[counter]);
    }
  printf("Sum of %d command line arguments is: %d\n", argc, sum);
}

atoi() documentation of a form.

Be forewarned, your code will still function if you input chars or strings into the command line, but the behavior will be odd. atoi() will convert the char to it's corresponding ASCII decimal character index/value.

You can also use a null value (argv[] is always terminated with a NULL value at argv[argc]) to end iteration. I prefer using the provided argc though.

Hopefully this helps you a little bit. If you don't understand anything I threw up here, comment or search it online.

Cheers!