realloc() triggering an exception

Question

I'm writing a generic function that receives an array (of any type) and finds the maximum value in that array (according to a specified compare() method), then returns a pointer array with pointers to all instances of that value in the specified array.

Problem is, when I try to realloc to increase the size of the pointer array result I see that my program triggered a breakpoint and if I press continue a few times, this is what happens:

Homework 5.3.exe has triggered a breakpoint.
HEAP[Homework 5.3.exe]: Heap block at 00806988 modified at 008069C0 past requested size of 2c
Homework 5.3.exe has triggered a breakpoint.
HEAP[Homework 5.3.exe]: Invalid address specified to RtlReAllocateHeap( 007F0000, 00806990 )
Homework 5.3.exe has triggered a breakpoint.
First-chance exception at 0x00F5464C in Homework 5.3.exe: 0xC0000005: Access violation writing location 0x00000020.
Unhandled exception at 0x00F5464C in Homework 5.3.exe: 0xC0000005: Access violation writing location 0x00000020.
The program '[7212] Homework 5.3.exe' has exited with code 0 (0x0).

This is my code:

void ** gMax(void * firstElement, void * lastElement, int sizeOfElement, int (*compare)(void * p1, void * p2))
{
    void ** result;
    int i, counter = 0;
    void * max = firstElement;
    if((result = (void **) malloc(sizeOfElement)) == NULL) return NULL;
    for(i = 0; i < ((char *)lastElement) - ((char *) firstElement); i += sizeOfElement)
    {
        printf("%d\n", *(int *)max);
        if(compare((char *)firstElement + i, max) > 0) max = (char *) firstElement + i;
    }
    for(i = 0; i < ((char *)lastElement) - ((char *) firstElement); i += sizeOfElement)
    {
        if(compare((char *)firstElement + i, max) == 0)
        {
            *(result + counter++ * sizeOfElement) = max;
            result = (void **) realloc(result, (counter + 1) * sizeOfElement);
        }
    }
    *(result + counter++ * sizeOfElement) = NULL;
    return result;
}
int main()
{
    int i;
    int a[] = { 2, 7, 5, 1, 7, 4, 7 };
    char b[][10] = { "xyz", "abc", "aaaa", "xyz" };
    int ** resultA = (int **) gMax(a, a + sizeof(a)/sizeof(*a), sizeof(*a), compareInt);
    char ** resultB = (char **) gMax(b, b + sizeof(b)/sizeof(*b), sizeof(*b), compareStringArray);
    return 0;
}

The breakpoint is triggered at the result = (void **) realloc(result, (counter + 1) * sizeOfElement);

What am I doing wrong?

EDIT: As a result of Skizz's answer, I changed my code to this:

void ** gMax(void * firstElement, void * lastElement, int sizeOfElement, int (*compare)(void * p1, void * p2))
{
    void ** result;
    int i, counter = 0;
    void * max = firstElement;
    if((result = (void **) malloc(sizeof(void *))) == NULL) return NULL;
    for(i = 0; i < ((char *)lastElement) - ((char *) firstElement); i += sizeOfElement)
    {
        printf("%d\n", *(int *)max);
        if(compare((char *)firstElement + i, max) > 0) max = (char *) firstElement + i;
    }
    for(i = 0; i < ((char *)lastElement) - ((char *) firstElement); i += sizeOfElement)
    {
        if(compare((char *)firstElement + i, max) == 0)
        {
            *(result + counter++ * sizeof(void *)) = max;
            result = (void **) realloc(result, (counter + 1) * sizeof(void *));
        }
    }
    *(result + counter++ * sizeof(void *)) = NULL;
    return result;
}

But it's still the same problem...

Thanks.

You are accessing memory beyond the bounds of the array. I'd recommend avoiding pre- and post-fix increments within your accesses, then step through the code with a debugger and/or valgrind and figure out where. — Dark Falcon, Jun 17 '14 at 16:27
Perhaps I wasn't quite clear enough in my answer, but the line `*(result + counter++ * sizeof(void *)) = max;` doesn't need the `sizeof(void*)` as this is done for you by the compiler (pointer arithmetic). — Skizz, Jun 17 '14 at 16:52
the third parameter to gmax() is expected to be the size of an element. However, what it is actually receiving is the size of a pointer (typically '4') — user3629249, Jun 18 '14 at 02:16

score 4 · Accepted Answer · edited Jun 17 '14 at 17:05

Your 'result' array is an array of pointers and not an array of items (int, char, etc). Your indexing into this array is therefore wrong:-

*(result + counter++ * sizeOfElement ) = max;

The highlighted item above is not required, pointer arithmetic already takes into account the size of the element (which in this case is a void *).

The other problem you have is in your memory allocation:-

result = (void **) malloc(sizeOfElement)

and

realloc(result, (counter + 1) * sizeOfElement)

You are not allocating arrays of type element, you're allocating arrays of pointers to elements (void *) so the sizeOfElement is not needed, rather, it should be sizeof (void *) instead.

score 1 · Answer 2 · edited May 23 '17 at 10:29

Your "edited" version still has the same bug.

Your code is:

*(result + counter++ * sizeof(void *)) = max;
result = (void **) realloc(result, (counter + 1) * sizeof(void *));

This code can be expressed without cruft and using index notation:

result[counter * sizeof(void *)] = max;
++counter;
result = realloc(result, (counter + 1) * sizeof(void *));

The indexing is wrong. You should be doing:

result[counter] = max;

The same problem occurs at the end;

*(result + counter++ * sizeof(void *)) = NULL;

should be

result[counter] = NULL;

Also you should check if the result of realloc is NULL and exit the program if so.

realloc() triggering an exception

2 Answers2