1

I am trying to make a simple example of a array that increases with its input. The input is a series of numbers and the end of this series is a zero. What I thought of was to increase my array every time a new number is read, but for some reason this does not seem to work since I get an error:

Realloc(): invalid pointer 

This is my current code:

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

int *resizeIntArray(int *series, int newSize) {
    int *newSeries = realloc(series, newSize * sizeof(int));
    if (newSeries == NULL) {
        printf("Error: Memory allocation failed");
        exit(-1);
    }
    return newSeries;
}

int main(int argc, char *argv[]) {
    int number;
    scanf("%d", &number);

    int *numbers;
    int size = 0;
    while (number != 0) {
       numbers = resizeIntArray(numbers, size + 1);
       printf("%d ", number);
       scanf("%d", &number);
       size++;
    }
}
2
  • 3
    Your compiler should have warned about this though. Which compiler are you using, and have you made sure to set sensible options? Commented Oct 29, 2016 at 10:22
  • You should test the return value of scanf() to avoid an endless loop at end of file if the file does not contain a 0 value. Commented Oct 29, 2016 at 15:49

4 Answers 4

3

You are passing an uninitialised variable to your function, in turn passed to realloc, which needs either a pointer to previously allocated memory, or NULL.

So initialise that variable:

int *numbers = NULL;
Sign up to request clarification or add additional context in comments.

2 Comments

@jurhas it is realloc that accepts the NULL pointer, in which case it behaves like malloc.
Note that realloc() may free the block of memory if passed a new size of 0 and return NULL or a pointer to an allocated object to which you are not allowed to write anything. Finally, contrary to popular belief, free also accepts NULL as an argument and does nothing.
1

There are multiple issues in your code:

  • you do not initialize numbers to NULL, so realloc() invokes undefined behavior the first time it is called. This is causing the problem you observe.
  • you do not check the return value of scanf(), causing a potential endless loop and undefined behavior if the input stream does not contain a 0 number.
  • you do not store the number in the reallocated array...
  • you do not free the array (minor).
  • you do not return 0 at the end of main() (minor).

Here a simpler and safer version:

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

int *resizeIntArray(int *series, int newSize) {
    int *newSeries = realloc(series, newSize * sizeof(int));
    if (newSeries == NULL) {
        printf("Error: Memory allocation failed");
        exit(-1);
    }
    return newSeries;
}

int main(int argc, char *argv[]) {
    int number;
    int *numbers = NULL;
    int i, size = 0;

    /* reading the numbers */
    while (scanf("%d", &number) == 1 && number != 0) {
       numbers = resizeIntArray(numbers, size + 1);
       numbers[size++] = number;
    }
    /* printing the numbers */
    for (i = 0; i < size; i++) {
       printf("%d ", numbers[i]);
    }
    printf("\n");

    free(numbers);
    return 0;
}

Comments

1

You can try an approach like this. This includes:

  • Memory checking, with appropriate error messages.
  • use of malloc() and realloc() to allocate and reallocate memory.
  • Allocates sufficient space when needed on run-time.
  • Appropriately checks return value of scanf().

Here is the code:

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

#define EXIT 0

void exit_if_null(void *ptr, const char *msg);

int
main(int argc, char const *argv[]) {
    int *numbers = NULL;
    int number, num_size = 1, count = 0, i;

    /* initial allocation of memory */
    numbers = malloc(num_size * sizeof(*numbers));

    /* small error checking, to be safe */
    exit_if_null(numbers, "Initial Allocation");

    /* Reading in numbers */
    printf("Enter numbers(0 to end): ");
    while (scanf("%d", &number) == 1 && number != EXIT) {

        /* valid number found, but is there space? */
        if (num_size == count) {
            num_size *= 2;

            /* resize run-time array */
            numbers = realloc(numbers, num_size * sizeof(*numbers));
            exit_if_null(numbers, "Reallocation");
        }
        numbers[count++] = number;
    }

    /* print out numbers */
    printf("Your numbers stored in array:\n");
    for (i = 0; i < count; i++) {
        printf("%d ", numbers[i]);
    }

    /* free allocated memory, very important */
    free(numbers);

    return 0;
}

/* helper function for error checking */
void
exit_if_null(void *ptr, const char *msg) {
    if (!ptr) {
        printf("Unexpected null pointer: %s\n", msg);
        exit(EXIT_FAILURE);
    }
}

Comments

0

First you should first allocate some memory before reallocate it. So your code will change as:

#include <stdio.h> 
#include <stdlib.h>
int *resizeIntArray(int *series, int newSize){
int *newSeries = realloc(series,newSize*sizeof(int));
if(newSeries == NULL){
 printf("Error: Memory allocation failed");
 exit(-1);
 }
return newSeries;
}
int main(int argc, char* argv[]) {
   int number;
   scanf("%d",&number);
   int *numbers=malloc(sizeof(int));///CHANGED
   int size = 1;///CHANGED
   while(number != 0){
   numbers = resizeIntArray(numbers,size +1);
   printf("%d ",number);
   scanf("%d",&number);
   size++;
 } 
}

But white you are doing is quite not efficent. A realloc() function hides a free() a malloc() and the worst one: memcpy(). So if you realloc() at each new item, you are going to have a bad time... O(n^2) exactly. the best way to do it is to allocate a buffer of memory:

  struct vector
  {   
      int *numbers;
      size_t size;
      size_t i;
   }
   #define DEFAULTBUF 100
   int main()
   {
          struct vector v;
          v.numbers=malloc(sizeof(int)*DEFAULTBUF);
          v.size=DEFAULTBUF;
          v.i=0;
          scanf("%d",&number);
          while(number != 0 && v.numbers){
             if (v.i->=v.size)
              {   v.size+=v.size
                 v.numbers=realloc(v.numbers,sizeof(int)*v.size);
               ///i leave to you the error handling
               }
               v.i++;
              printf("%d ",number);
              scanf("%d",&number);

             }  

   }

The correct use of realloc() malloc() and similar is very important. And also the increasing ratio of the resize. For the datastructure I ever double it. For text I proceed linearly

2 Comments

Actually, O(n^2) is not guaranteed. It will depend on the actual implementation of malloc() and friends. I have seen some that only use blocks with sizes all powers of 2. Calling realloc() repeatedly in this case has the same effect and complexity as you solution. Furthermore, it is more important to handle out of memory properly and dispose of allocated memory correctly than to focus on optimisation too early. Of course the free is not mandatory because the program exits immediately, but it is a good habit to take from the beginning.
It can work with small array, but I wouldn't base a code over these considerations. When I wrote the code for my btree , I did the benchmark for 3.000.000 items. The reallocating step was linear with blocks of 4096 nodes. It required 25 seconds. Just changing the line bt->sz+=DEFAULTBLOCKSIZE; to bt->sz+=bt->sz; the time sinked to 3 seconds. Since the day I am ever very carefully with realloc() and friends. Ever prefer list to vector, unless the vector is static or quasi-static, ever allocate some buffer more and so on... if a program requires 3 or 3.1 Mb nobody is going to notice, but 25 s

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.