C malloc in function throw memory error

Question

I have a problem with my function.

char *readFnName(char *fnString, int n, int offset, int *size) {
    char *fnName;
    int nameBuffer, i, j;

    i = offset;
    j = 0;
    nameBuffer = 8;
    fnName = (char *)malloc(nameBuffer);

    while(*(fnString + i) != '(' && i<n) {
        *(fnName + j++) = *(fnString + i++);
        if (j>=nameBuffer) {
            nameBuffer += 8;
            fnName = (char *)realloc(fnName, nameBuffer);
        }
    }

    *(fnName + j++) = '\0';
    *size = j;

    return fnName;
}

I add this function a string, and this is read it well, but i run this function in a loop, and in the 3rd iteration this line crushed:

fnName = (char *)malloc(nameBuffer);

I get a message in netbeans debugger:

malloc(): memory corruption: 0x08a62218

I use netbeans and lubuntu.

Can you please remove all those pointless casts? They make the code ever so much uglier to read. — Kerrek SB, May 11 '14 at 21:44
[don't cast malloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) . @self, if you have an issue with this advice, maybe start a meta thread on it instead of spamming comments . I don't think it's a coincidence that there are 500+ upvotes — M.M, May 11 '14 at 21:44
@MattMcNabb You hit the problem right there with the word `spamming` and who is doing it really. — this, May 11 '14 at 21:47
@self.: The advice not to cast the result of `malloc` is relevant to this question. Your comments are not. — Keith Thompson, May 11 '14 at 21:48
@DevMetal91, the code looks OK, apart from failing to check whether the alloc functions returned `NULL`, and possible failure to include `stdlib.h`. If you still have problems then perhaps the issue is elsewhere in your code ,e.g. heap corruption, and it is just happening to show up when you do this malloc — M.M, May 11 '14 at 21:49
Or a less inflammatory one: http://c-faq.com/malloc/mallocnocast.html — P.P, May 11 '14 at 21:49
@MattMcNabb, i use this inside one recursive function, so the heap corruption is sounds possible to me, what can i do againts that? — DevMetal91, May 11 '14 at 22:02
Try running valgrind on the whole program, it may show something up. Failing that, check carefully over code that is using malloc'd buffers to make sure it does not overflow them. You may have to divide-and-conquer to figure out which part of the program has the problem. — M.M, May 11 '14 at 22:03
@MattMcNabb, Thank you for the help. You was right, the problem was somwhere else. This valgrind is good stuff. Thank you for the help, u saved my night :) — DevMetal91, May 11 '14 at 22:39

score 2 · Accepted Answer · answered May 11 '14 at 23:02

There's no problem with the code you posted. The runtime error message indicates that there may be heap corruption earlier on in the program, which is being shown up by a later call to an *alloc function.

Try running valgrind on the whole program, it may find something. Failing that, check carefully over code that is using malloc'd buffers to make sure it does not overflow them. You may have to divide-and-conquer to figure out which part of the program has the problem.

score 1 · Answer 2 · edited Jun 04 '15 at 05:49

 fnName = (char *)realloc(fnName, nameBuffer);

realloc extends your memory by re-allocating more memory for you. If the extra memory that you asked for cannot be given in the same contiguous area that your old fnName memory is located, the allocator will go grab your allocation block from another place in memory. This means that all of the pointers still pointing to the old fnName memory area (for example another pointer pointing to *(fnName + N)), are still pointing there even though the allocator gave you a new address for fnName so that you could have all of the memory that you requested. Those pointers are called dangling pointers, because they point to de-allocated memory that the program does not own anymore. This is why you get the "memory corruption" message. The OS is telling you that your program tried to access memory that it does not own. This happens when your program uses those dangling pointers as if they still pointed to allocated memory.
realloc works fine unless you ask for more memory than the allocator can provide contiguously from your current address. If it can't provide the memory contiguously, it will go grab it from somewhere else and return another memory address to you.
To fix this, you have to reassign all the pointers still referencing the old memory that is re-allocated to the new memory address. Since you can't really predict when you need to do this and you don't want to write unnecessary code, and since C does not have classes, you should consider allocating a large enough memory pool in the beginning of your program. Then you can write a simple allocation/de-allocation algorithm that allows you to grab memory from it during your functions. Doing this will teach you more about allocating contiguous memory.

    char* pool;
    if ((pool = malloc(sizeOfPoolInBytes)) == NULL)
      merror();

Take a look at your modified function below:

#include<stdlib.h>
#include<iostream>
using namespace std;

char* readFnName(char* fnString, int n, int offset, int* size){
  char *fnName = NULL;
  int nameBuffer = 8;
  int i = offset;
  int j = 0;
  int count = 0;
  int fnNameIndex = 0;

  //-------------------------------------------------------- Since you want to copy everything but '(' find out how 
  //-------------------------------------------------------- long fnString is without all the '(' characters.
  for( int k = 0; k < n; k++)
  {
    if( *(fnString + k) != '(' )
    {
      count += 1;
    }
  }

  //-------------------------------------------------------- Now, just allocate a string to store the result
  fnName = malloc(count);

  //-------------------------------------------------------- Now, copy everything but the '(' character
  for( int p = i; p < n; p++){
    if( *(fnString + p) != '(')
    {
      if(fnNameIndex < count)
        {
          *(fnName + fnNameIndex) = *(fnString + p);
          fnNameIndex++;
        }
    }
  }

  *size = fnNameIndex;//----------------------------------- This will give the accurate size in case the offset is not = 0
  return fnName;
}

int main(){

  char* string = NULL;
  int n = 5;
  string = malloc(n);
  string[0] = 'a';
  string[1] = 'b';
  string[2] = '(';
  string[3] = 'c';
  string[4] = '\0';

  char* newString = NULL;
  int newStringSize = 0;
  int* ptrSize = &newStringSize;
  newString = readFnName(string, n, 0, ptrSize); 

  for(int i = 0; i < n; i++){
    printf("%c\n", string[i]);
  }

  printf("\n");

  for(int j = 0; j < *(ptrSize); j++){
    printf("%c\n", newString[j]);
  }

  free(string);
  free(newString);

  return 0;
}

fnName = malloc(count) will allocate count number of bytes as a void pointer. This example needs a character pointer. It also needs malloc to allocate memory that can hold character variables. using malloc(count) will work if your OS represents characters as 1 byte. It is best practice to multiply count by the size of the variable that you are allocating for. fnName = (char*) malloc(sizeof(char)*count); — Taruchu, Jun 07 '15 at 02:23
This way if this code is compiled on a different operating system that represents character variables as 2 bytes, the code will always grab the size of character that the OS uses (2 bytes) and multiply that by count. This way malloc will allocate enough memory. If you were to use just count in this case, when a character variable is 2 bytes, your program would not have enough memory to hold the character string that you need, and eventually an memory "access violation" error would occur when you try to process the string. — Taruchu, Jun 07 '15 at 02:33
I know that in this case you could use malloc(count), but that would not work for count number of integers or count number of structure types. You would need malloc(sizeof(int) * count) or malloc(sizeof(struct) * count). So it is best practice to use fnName = (variable pointer) malloc(sizeof( variable type ) * count of variables). — Taruchu, Jun 07 '15 at 02:33
The sizeof() function is handy because it finds the size in bytes of a variable type. This is the size in bytes that the OS you are running uses for that variable type. — Taruchu, Jun 07 '15 at 02:36

C malloc in function throw memory error

2 Answers2

Linked