0

I'm trying to copy an array of integers in a dynamically allocated array. At first it has size 1 and with every element I want to increase it's size by one.

Here's the code: 

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

int main()
{
    int *nr,i;
    nr=(int *)malloc(1*sizeof(int));
    for(i=1; i<1000; i++)
    {
        nr[i]=i;
        nr=(int *)realloc(nr,i*sizeof(int));

    }

    for(i=1; i<1000; i++)
        printf("%d \n", nr[i]);

    return 0;
}

Thanks!

user3623498
  • 69
  • 1
  • 1
  • 8
  • 2
    The second parameter to `realloc()` is not "how large to increase it by", its "the absolute size to set it to" -- your call is therefore not trying to change the size at all -- which is invalid for this function. – mah May 10 '14 at 14:36
  • I was wrong, `nr=(int *)realloc(nr,i*sizeof(int));` – user3623498 May 10 '14 at 14:44
  • `for(i=1;` don't you like element 0? The `malloc()` made room for `nr[0]` but you immediately write into `nr[1]`; undefined behavior from that. You continue to write one beyond the size of your buffer, and after a few loops you've overwritten something the malloc library needed to maintain your buffer; `realloc()` needed that data that you overwrote, leading to the error. +1 to @Salgar's answer. – mah May 10 '14 at 14:52

1 Answers1

2

Here: nr=(int *)malloc(1*sizeof(int));

You are only allocating 4 bytes, yet in your for loop you're then writing 4000 bytes, 3996 of which is in memory not allocated to you. And the realloc you're doing isn't doing anything because you're still only asking for 4 bytes.

Kill the realloc, and just allocate 1000*sizeof(int) at the begining.

Edit for your edit:

Because your loop is from i=1, on the first iteration you're writing in bytes 4-7 instead of 0-3, you need to iterate from i=0. But your realloc is also wrong, because even with your loop you're allocating 1*sizeof(int), which means you'd still be one short. You need to loop from int i=0 and realloc with (i+2)*sizeof(int).

BUT

Don't realloc in a loop, malloc and realloc are very slow. If you did this with a real problem with large numbers you would have a bottleneck here. Allocate once, use it. Even if you can't do it like that, calculate how much space you need in a loop, do a single malloc, then use another loop to fill it in, it will be significantly faster.

Salgar
  • 7,687
  • 1
  • 25
  • 39