CODEWITHSUNDEEP
cdynamic-memory-allocation
Renan Gemignani
Renan Gemignani

Reputation: 2793

realloc fails after multiple calls only when not debugging

The below code occasionally fails on the buffer = (char*) realloc(buffer, allocated * sizeof(char)); call (marked down below) that I use to dynamically allocate space for a char*,by allocating 1 char initially, and doubling the allocated amount every time the memory I already have is insufficient to store the string.

I have very similar code in many other parts of my project, with the same memory allocation policy and calls (changing only the type of the void* I pass to realloc).

I am using VS2010 to debug the problem, and when I start the program on debug mode, the function always completes successfully.

However, when calling the program from the command line, there is a good chance that one of the calls to realloc will fail after some time with an "Access violation reading location" error - though it doesn't happen all the time, and only happens after the function below has been called multiple times, with many reallocations having already taken place.

What's weirder, I put some prints before and after the realloc call to assert if the pointer location was changed, and, when I did so and ran the program, the calls to realloc stopped failing randomly.

What am I doing wrong?

TOKEN
next_token_file(FILE* file, 
                STATE_MACHINE* sm, 
                STATE_MACHINE* wsssm)
{
    char* buffer = (char*) malloc(sizeof(char));
    size_t allocated = 1;
    size_t i = 0;
    while(1)
    {
    /*
    ... code that increments i by one and messes with sm a bit. Does nothing to the buffer.
    */
        // XXX: This fails when using realloc. Why?
        if(i + 1 >= allocated)
        {
            allocated = allocated << 1;
            buffer = (char*) realloc(buffer, allocated * sizeof(char));
        }
        buffer[i] = sm->current_state->state;
    /*
    ... more code that doesn't concern the buffer
    */
    }
    // Null-terminate string.
    buffer[++i] = 0;
    TOKEN t = {ret, buffer};
    return t;
}

Upvotes: 1

Views: 2826

Answers (3)

alk
alk

Reputation: 70971

Due to these lines

char* buffer = (char*) malloc(16 * sizeof(char));
size_t allocated = 1;

the program shrinks buffer for the first 4 re-allocations. So the program writes to unallocated memory from i=16 on, which is undefined behaviour, so anything could happen. Also this most likely smashes the memory management which in turn makes realloc() fail.

You might like to change those two lines to be:

size_t allocated = 16; /* or = 1 if the 16 was a typo. */
char * buffer = malloc(allocated);

Other notes:

Refering the last note, the following modifications should be applied

char * buffer = malloc(allocated);

might become:

char * buffer = malloc(allocated); 
if (NULL == buffer)
{
  /* Error handling goes here. */
}

and

buffer = (char*) realloc(buffer, allocated * sizeof(char));

might become:

{
  char * pctmp = realloc(buffer, allocated);
  if (NULL == pctmp)
  {
    /* Error handling goes here. */
  }
  else
  {
    buffer = pctmp;
  }
}

Upvotes: 4

dhein
dhein

Reputation: 6555

I dont know, when you are increasing or decreasing i. But I would bet, acording to this snippet, your problem is: your reallocating infinitly, and as your not checking for realloc is returning NULL, that will crash your programm ;)

As allready said, even the not well running pritf's are conforming it, your violating your memory block. this will happen by reallocing the memory adress which has been overwritten outside the range.(excepting its UB anyway)

Or if you try to work if an invalid return value (what is when NULL is returned, what could happen because u aren't checking it) Or if you request zerosized area(size parameter is 0) and you get returned an non zero pointer and you work with that one. But 2nd case probably wont happen in your programm ;)

Upvotes: 0

Charlie Burns
Charlie Burns

Reputation: 7044

More of a comment than an answer but I don't have 50 points to comment.

This:

char* buffer = (char*) malloc(16 * sizeof(char));

should be

char* buffer = (char*) malloc(1 * sizeof(char));

or 

allocated = 16.

Upvotes: 1

Related Questions