Access violation on 2nd iteration of for loop when accessing a 2D array-CodePudding

I have an assignment in C where the user has to populate a 2D grades[3][3] array. Then a function is called that returns a pointer to main() with the address of the max element in that array.

So, I've defined a int *get_max() function with the following body:

int *get_max(float *gradesPtr)
{
    int i, j, *max_addr = &i; //variable initialization
    float max = *gradesPtr, curr_element; //max gets the value of the 1st element in the array to not be empty

    for (i = 0; i < 3; i  )
    {
        for (j = 0; j < 3; j  )
        {
            curr_element = *(gradesPtr   i*3   j); //(pointer   i*columns   j) formula used here
            if (curr_element > max)
            {
                max = curr_element; //get max element
                *max_addr = gradesPtr   i*3   j; //get address of max element
            }
        }
    }

    return max_addr; //return "max_addr" to main
}

It's called from main() like this:

int *max_element = get_max((float *)grades);

The code seems correct, since I can verify through the Watch menu in Visual Studio that it runs fine on the 1st iteration, and that the variables have the correct values. On the 2nd iteration, though, instead of continuing normally, it throws an access violation error on the curr_element = *(gradesPtr i*3 j); line.

After debugging it for a bit, it seems that no matter how I pass the array to the function (using a pointer to the array or passing the whole array itself) and no matter how I save the address of the max element to *max_addr (either by using the (pointer i*columns j) formula or by &grades[i][j]), the get_max() function sees the array as a 1D array instead of a 2D one, thus throwing the access violation error. Searching here provided some good-looking solutions, but they were not what I wanted.

Is there anything I'm missing here?

CodePudding user response：

If grades is a two-dimensional array, you should define the parameter and return value appropriately

float *get_max(float gradesPtr[3][3])

Now there's no need anymore to do pointer arithmetic and you may simply say

float *get_max(float grades[3][3])
{
    int i, j;
    float *max_addr = &grades[0][0]; //variable initialization
    float max = *max_addr;

    for (i = 0; i < 3; i  )
    {
        for (j = 0; j < 3; j  )
        {
            float curr_element = grades[i][j];
            if (curr_element > max)
            {
                max = curr_element; //get max element
                max_addr = &grades[i][j]; //get address of max element
            }
        }
    }

    return max_addr; //return "max_addr" to main
}

CodePudding user response：

In get_max, you do:

int i, *max_addr = &i;

Thus, max_addr is pointing [initially] to the function scoped variable i.

If the if (curr_element > max) is never true, max_addr will never be updated. The function will return a max_addr that points to the stack.

This is UB (undefined behavior) because i goes out of scope.

The if will fail to be true if the first element of the array (e.g. gradesPtr[0] aka grades[0][0]) is the maximum.

And, inside the if, you're changing what max_addr points to rather than updating max_addr.

So, you want to change (which won't compile):

*max_addr = gradesPtr   i * 3   j;

Into:

max_addr = gradesPtr   i * 3   j

max_addr is a float * but the return value of the function is int *. So, I'm not sure how this compiles.

Instead of:

int *max_addr = &i;

You want:

float *max_addr = gradesPtr;

And, you want to change the return value to float *

Here's the refactored code:

float *
get_max(float *gradesPtr)
{
    int i;
    int j;
    float *max_addr = gradesPtr;
    // max gets the value of the 1st element in the array to not be empty
    float max = *gradesPtr;
    float curr_element;

    for (i = 0; i < 3; i  ) {
        for (j = 0; j < 3; j  ) {
            // (pointer   i*columns   j) formula used here
            curr_element = *(gradesPtr   i * 3   j);

            // get max element and address of max element
            if (curr_element > max) {
                max = curr_element;
                max_addr = gradesPtr   i * 3   j;
            }
        }
    }

    return max_addr;
}

Since you're using a 1D pointer in the function and are returning a pointer we can treat the array as 1D:

float *
get_max(float *gradesPtr)
{
    float *max_addr = gradesPtr;
    float max_val = *max_addr;
    float *end_ptr = gradesPtr   (3 * 3);
    float *cur_ptr;

    for (cur_ptr = gradesPtr;  cur_ptr < end_ptr;    cur_ptr) {
        if (*cur_ptr > max_val) {
            max_val = *cur_ptr;
            max_addr = cur_ptr;
        }
    }

    return max_addr;
}