1

I'm trying to create a dynamic array in C to implement a queue, when I compile I get the following error: * glibc detected . / Ex: realloc (): invalid next size: 0x0000000001fbe030 **

My code is as follows:

typedef uint32_t u32;
typedef uint64_t u64;


typedef struct _queue_t *queue_t;

struct _queue_t {
    u32 *array; // Arreglo para guardar elementos de la cola
    u32 FirstElem; // Primer elemento de la cola
    u32 LastElem; // Ultimo elemento de la cola
    u32 memory_allocated; // Para saber si tengo que pedir mas memoria
    u32 Size; // Devuelve la cantidad actual de elementos que tiene la cola
};

queue_t queue_empty() {
    queue_t queue = NULL;
    queue = calloc (1, sizeof (struct _queue_t));

    assert(queue != NULL);

    queue->array = (u32 *) calloc(100, sizeof(u32));
    queue->FirstElem = 0;
    queue->LastElem = 0;
    queue->memory_allocated = 100;
    queue->Size = 0;

    return queue;
}

int main() {

    queue_t queue = NULL;
    queue = queue_empty();

    for (u32 i = 0; i < 1000; i++) {

        if (queue->memory_allocated == queue->Size) {
            queue->array = (u32 *) realloc (queue->array, 100*sizeof(u32));
            queue->memory_allocated += 100;
        }

        queue->LastElem += 1;
        queue->array[queue->LastElem] = i;
        queue->Size += 1;
    }

    return(0);
}

Why this error occurs?

pabloapast
  • 75
  • 8
  • Your code has a bug - if `realloc()` fails, it returns a NULL pointer, which would overwrite your `queue->array` variable and the original memory block would leak (since you wouldn't be able to free it up anymore). – xxbbcc Jun 08 '13 at 03:16

1 Answers1

2

You always allocate the same size of memory here:

if (queue->memory_allocated == queue->Size) {
  queue->array = (u32 *) realloc(
      queue->array, 100*sizeof(u32)); // always 100 * sizeof(u32)
  queue->memory_allocated += 100;
}

I think what you want to do is to allocate 100 elements more. You should also check the return value of realloc() by introducing a temporary to store it in, since NULL might be returned, resulting in losing the only pointer to the currently allocated memory:

if (queue->memory_allocated == queue->Size) {
  int new_size = queue->memory_allocated + 100; // increment first
  u32* new_array = (u32 *) realloc(queue->array, new_size * sizeof(u32));
  if (new_array) { // update only if realloc() returns a valid address.
    queue->memory_allocated = new_size;
    queue->array = new_array;
  } else {
    // do something in react
  }
}
Nikos C.
  • 50,738
  • 9
  • 71
  • 96
keelar
  • 5,814
  • 7
  • 40
  • 79
  • 2
    Your code has the same bug as the original. You need to first put the pointer returned by `realloc()` in a temporary variable because if it's NULL, the original would be overwritten. – xxbbcc Jun 08 '13 at 03:20
  • 1
    I'm not sure the OP realizes that his "memory_allocated" is not actually the size in bytes; the name doesn't imply it. – kfsone Jun 08 '13 at 03:20
  • @xxbbcc Indeed. Though the answer does correctly address why the program fails at runtime. – Nikos C. Jun 08 '13 at 03:21
  • @NikosC. Yes, I didn't say anything about the rest of the answer. It is, however, a common bug to forget to check the return value of memory allocation. I've seen many serious errors because of this. – xxbbcc Jun 08 '13 at 03:23
  • @xxbbcc, thank you for pointing this out. I have added check for the return value of realloc(). – keelar Jun 08 '13 at 03:26
  • @keelar Yes, I saw it. +1 for your answer. – xxbbcc Jun 08 '13 at 03:27
  • Though you can just integrate it directly in your answer. There's no need to slap it on with an "EDIT:". It's best if answers don't have the edit history embedded in their text. – Nikos C. Jun 08 '13 at 03:29
  • Ok, Thank you very much. But I have another question: Why failure realloc ()? If realloc () fails, the rest of the execution also fail, this is just a small piece of the original code. As I can do to create a dynamic array, where you can allocate more space when you need it? – pabloapast Jun 08 '13 at 03:44
  • @NikosC. Thank you for your tips. I added since I sometimes saw people do this. I have removed it from my answer. – keelar Jun 08 '13 at 03:47
  • @userpap: Yes, failure in realloc() usually indicates insufficient memory, but it is nice to check the return value to prevent from corrupting memory or illegal memory access. – keelar Jun 08 '13 at 03:50
  • @userpap `realloc()` may also fail if there's memory corruption, a hardware issue or there may be other situations where it may fail that are not even obvious. Don't skip on checking return values - especially not on memory allocation functions. If your code has a memory corrupting bug, you'll have to go through hell to find it if you skip these kind of safety checks. @keelar makes it sound like it's a nice-to-have but there's no way to stress enough how important it is to do so. – xxbbcc Jun 08 '13 at 16:58