My C program outputs undesired array values

Question

I've been tasked to code a program that processes a simple 1D array to return its element values, but the compiler has been behaving strangely; outputting more values than I have array elements.. It's also not being fully compliant with one of my statements (one that prints a new line character every 8 elements) and not assigning the largest value to my variable. I think that the other two problems will go away once the first problem is fixed, however.

Here is my brief:

Design, code and test a program that:

1. Fills a 20 element array (marks) with random numbers between 0 and 100.
2. Prints the numbers out 8 to a line
3. Prints out the biggest number, the smallest number and the average of the numbers

And here is my code:

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

int main(){

    srand(time(NULL));

    int marks[20];
    int i = 0;
    int sum = 0;
    int min;
    int max;

    for(i;i<=sizeof(marks);i ++){

        marks[i] = rand() % 100;
        sum += marks[i];

        if(i % 8 == 0){
            printf("\n");
        }

        printf("%d ", marks[i]);
        if(marks[i]>max){
            max = marks[i];
        }
        else  if(marks[i]<min){
            min = marks[i];
        }
    }


    printf("\n\nThe minimum value is: %d", min);
    printf("\nThe maximum value is: %d", max);
    printf("\n\nThe average value is: %d", sum / sizeof(marks));

    return 0;
}

Please can someone help me get the correct output?

Answer 1

sizeof() function returns the byte length of the array, so this code "thinks" your array is 20 * whatever byte size ints are on your machine. You will want to just use i < 20 in the loop or go

for (i;i<sizeof(marks)/sizeof(int); i ++) { ...

Note that you probably do not want the <= operator in the for loop, since arrays are 0 indexed, thus marks[20] is actually one beyond the array.

Answer 2

There are two problem I can see that will invoke undefined behavior in your code.

1. By saying for(i;i<=sizeof(marks);i ++), you're out of bounds.
2. int min; int max; are not initialized and you're attempting to use it.

to solve this.

1. Change the for loop condition to for(i; i< 20; i++). Better to use a preprocessor construct like #define SIZ 20 and then make use of it accross your code to make it consistent and robust.
2. Initialize your local variables. max should be INT_MIN, and min can be INT_MAX. (see limits.h for reference).

---

To clarify more on point 2, max and min are automatic local variables, and in case not initialized explicitly, it contains indeterminate values.

C11, chapter §6.7.9,

If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate.

and then, directly from the Aneex J, §J.2, Undefined behaviour,

The value of an object with automatic storage duration is used while it is indeterminate.

Answer 3

if(marks[i]>max){
    max = marks[i];
}
else  if(marks[i]<min){
    min = marks[i];
}

min and max are not initialized here. Make sure to set your compiler warnings at the highest level, so you get a warning message when you forget to initialize variables.

for(i;i<=sizeof(marks);i ++){

This doesn't make sense. Replace sizeof(marks) with the number of times you want to loop, and use < instead of <=.

For example:

const int num_marks = 20; // or use #define
int marks[num_marks];
for(i = 0; i < num_marks; i++) {}