Search code examples
cfor-loopfgets

fgets causing an infinite for loop in C


It seems my implementation of fgets() is incorrect here, would very much appreciate some extra eyes to look over what I've done!

Here's the code

int main(int argc, const char* argv[]){
    int numIntegers;
    char buffer[20];
    int intArray[10];
    //if no argument is passed in, terminate
    if (argc == 1){
            printf("no argument given, terminating..\n");
            return EXIT_FAILURE;
    }
    else{
            numIntegers = atoi(argv[1]);
            //we only want numbers greater than 0
            if (numIntegers <= 0){
                    printf("# must be greater than 0\n");
                    return EXIT_FAILURE;
            }
            else{
                    printf("Enter %d integer values to place in array: \n", numIntegers);
                    for (int i = 0; i < numIntegers; i++){
                            fgets(buffer, numIntegers, stdin);
                            intArray[i] = atoi(buffer);
                            printf("Index is = %d \n", i);
                    }
            }
    }

    //for (int i =0; i < numIntegers; i++){
    //      printf("Index[%d] = %d \n", i, intArray[i]);
    //}
}

Here's the output, the line with no other text besides an integer is user input. Notice how the value of i resets. The issue only occurs when I give an initial argument of anything more than 10. It turns the for loop into an endless loop, for whatever reason.

$ ./a.out 11
Enter 11 integer values to place in array:
5
Index is = 0
2
Index is = 1
1
Index is = 2
2
Index is = 3
3
Index is = 4
4
Index is = 5
123
Index is = 6
123
Index is = 7 
123
Index is = 8
1
Index is = 9
2
Index is = 2
2
Index is = 3
3
Index is = 4
5
Index is = 5
1
Index is = 6
12
Index is = 7

Solution

  • You are using

    fgets(buffer, numIntegers, stdin);
    

    The second parameter should be the size of the buffer - in your case, 20. That is at least one obvious problem...

    The next problem: you are allowing numIntegers to be greater than 10 - so you will be writing values beyond the end of your intArray. Need to fix that too...

    if(numIntegers > 10) {
      printf("cannot have number greater than 10!\n");
      // abort, retry, ignore...
    }
    

    In fact - here is your code, with the bugs ironed out: note the use of defined sizes for BUFSIZE and MAXNUM just so you don't have to change it in multiple places if you change your mind...

    #include <stdio.h>
    #define BUFSIZE 20
    #define MAXNUM 10
    #define EXIT_FAILURE 0
    
    int main(int argc, const char* argv[]){
        int i, numIntegers;
        char buffer[BUFSIZE];
        int intArray[MAXNUM];
        //if no argument is passed in, terminate
        if (argc == 1){
                printf("no argument given, terminating..\n");
                return EXIT_FAILURE;
        }
        else{
                numIntegers = atoi(argv[1]);
                //we only want numbers greater than 0
                if (numIntegers <= 0 || numIntegers > MAXNUM){
                        printf("# must be greater than 0 and less than %d!\n", MAXNUM);
                        return EXIT_FAILURE;
                }
                else{
                        printf("Enter %d integer values to place in array: \n", numIntegers);
                        for (i = 0; i < numIntegers; i++){
                                fgets(buffer, BUFSIZE, stdin);
                                intArray[i] = atoi(buffer);
                                printf("Index is = %d \n", i);
                        }
                }
        }
     }
    

    Finally - you may wonder why your integer counter seems to "reset"? Well - your intArray is a block of 10 integers on the stack; and when you declare loop variable i, it occupies the next place in memory (as int intArray[10]; was the last time a variable was declared before you got to the for loop) - which you happen to get to when you "index" to intArray[10] (a memory location you are not allowed to access, but you did anyway). You happened to enter the value 2 - and thus, i was reset to 2...

    If you had declared i at the start of the program (as I did, since my compiler doesn't "do" C99 by default - I'm that old!), the problem would have shown up differently - or not at all.