Seemingly undefined behavior with realloc in C prime generator (program posted, runnable)

Question

I wrote a program to find prime numbers and I use realloc() to change the size of the '*primes' block. When I put any number of bits greater than 6, I find that the first two values seem like undefined behavior and I cannot for the life of me understand why this happens (the numbers that follow are all correct). pls halp.

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

typedef int int64_t;

void *safe_calloc(size_t,size_t);
void *safe_realloc(void*,size_t);

int main()
{
    printf("Prime Generator\nPlease enter the number of usable bits: ");
    int bits =0, maxNum=1, size = 6, elems = 3;

    scanf("%d",&bits);

    if(bits<1)
        exit(0);

    int i=0;
    for(; i < bits; i++)
        maxNum*=2;

    int *primes = safe_calloc(size*sizeof(int),sizeof(int));
    int *temp = safe_calloc(size*sizeof(int),sizeof(int));

    primes[0] = 1;
    primes[1] = 2;
    primes[2] = 3;

    int n = 3,j;
    for(; n < maxNum; n+=2){

        for(j = 2; j < elems; j++){

            if(n%primes[j] == 0)
                break;
            else if(j == elems-1){

            primes[elems++] = n;
            if((size-elems) < 2){

                for(i = 0; i < elems; i++)
                    printf("%d\n",primes[i]);
                printf("\n");

                size += 8;  // add 8 spaces to the prime storage array
                temp = safe_realloc(primes,size*sizeof(int));

                for(i = 0; i < elems; i++)
                    printf("%d\n",primes[i]);
                printf("\n");

                memmove(temp,primes,(size-8)*sizeof(int)); // copy over data to new array, just to be sure
                primes = temp;

                for(i = 0; i < elems; i++)
                    printf("%d\n",primes[i]);
                printf("\n");
                }
            }
        }
    }
    if(bits == 1){
        printf("1");
    }
    else{
        for(i = 0; i < elems; i++)
            printf("%d\n",primes[i]);
        printf("\n");
    }
    free(temp);
    free(primes);

    return 0;
}
void *safe_calloc(size_t length,size_t dataSize){
    int *tmp;
    if ((tmp = calloc(length,dataSize)) == NULL) {
        printf("ERROR: calloc failed");
        exit(0);
    }
    return tmp;
}
void *safe_realloc(void* ptr, size_t arraySize){
    int *tmp;
    if ((tmp = realloc(ptr,arraySize)) == NULL) {
        printf("ERROR: realloc failed");
        exit(0);
    }
    return tmp;
}

Note: An `exit` code of `0` is commonly used to report "no error" (that`s also what `main` returns implicitly). You should return a small positive integer instead, typically `1` if no other values are used. — too honest for this site, Apr 02 '16 at 20:20
1) Not the error: `int *primes = safe_calloc(size*sizeof(int),sizeof(int));` is probably a bit too large ... 2: `primes[0] = 1;` I don't think this a good idea. — wildplasser, Apr 02 '16 at 21:06

score 1 · Answer 1 · answered Apr 02 '16 at 20:26

1

You don't need to memmove the data. realloc does that automatically and frees the old buffer. So the memmove is copying from an invalid (already freed) memory space.

answered Apr 02 '16 at 20:26

user3386109

34,287
7
49
68

score 1 · Accepted Answer · answered Apr 02 '16 at 20:26

Do not use the pointer you put into realloc after the call - only use the returned pointer. You also do not need to copy/memmove:

The contents of the object shall remain unchanged up to the lesser of the new and old sizes. If the new size of the memory object would require movement of the object, the space for the previous instantiation of the object is freed.

ryyker · Answer 3 · 2016-04-02T21:38:43.623

0

As others have pointed out, using memmove in the way you are is not correct. realloc preserves the current content, and adds additional memory so you can then continue with strcat or strcpy as needed.

For realloc() to prevent memory loss, use a temporary buf, check for NULL, then reassign to original...

temp = realloc(temp, 20);
if(!temp)
{
     //handle error and exit
}
buf = temp;//transfer successfully allocated memory
....
//use newly reallocated buf

edited Apr 02 '16 at 21:38

answered Apr 02 '16 at 20:39

ryyker

22,849
3
43
87

@user3386109 - I did read them, and although you are checking for null, you are not using `realloc()` in the correct way. `memmove()` is not necessary. By mentioning this feature of `realloc()`, I did not want to convey it is any less important to also check return of the allocation functions in general. _But yes, this you have done correctly!_ :) However, the method you employ with `realloc()` in conjunction with `memmove()` will cause you problems. – ryyker Apr 02 '16 at 21:17

Seemingly undefined behavior with realloc in C prime generator (program posted, runnable)

3 Answers3