Search code examples
cmicrocontrollerlpc

Trying to free memory after calling strcpy causes the program to crash


I understand that a lot of people here complain about strcpy, but I haven't found anything using search that addresses the issue I have.

First off, calling strcpy itself doesn't cause any sort of crash/segmentation fault itself. Secondly, the code is contained within a function, and the first time that I call this function it works perfectly. It only crashes on the second time through.

I'm programming with the LPC1788 microcontroller; memory is pretty limited, so I can see why things like malloc may fail, but not free.

The function trimMessage() contains the code, and the purpose of the function is to remove a portion of a large string array if it becomes too large.

void trimMessage()
{
  int trimIndex;
  // currMessage is a globally declared char array that has already been malloc'd
  // and written to.
  size_t msgSize = strlen(currMessage);

  // Iterate through the array and find the first newline character. Everything
  // from the start of the array to this character represents the oldest 'message'
  // in the array, to be got rid of.
  for(int i=0; i < msgSize; i++)
  {
    if(currMessage[i] == '\n')
    {
      trimIndex = i;
      break;
    }
  }
  // e.g.: "\fProgram started\r\nHow are you?\r".
  char *trimMessage = (char*)malloc((msgSize - trimIndex - 1) * sizeof(char));

  trimMessage[0] = '\f';

  // trimTimes = the number of times this function has been called and fully executed.
  // freeing memory just below is non-sensical, but it works without crashing.
  //if(trimTimes == 1) { printf("This was called!\n"); free(trimMessage); }
  strcpy(&trimMessage[1], &currMessage[trimIndex+1]);

  // The following line will cause the program to crash. 
  if(trimTimes == 1) free(trimMessage);
  printf("trimMessage: >%s<\n", trimMessage);

  // Frees up the memory allocated to currMessage from last iteration
  // before assigning new memory.
  free(currMessage);
  currMessage = malloc((msgSize - trimIndex + 1) * sizeof(char));

  for(int i=0; i < msgSize - trimIndex; i++)
  {
    currMessage[i] = trimMessage[i];
  }

  currMessage[msgSize - trimIndex] = '\0';
  free(trimMessage);
  trimMessage = NULL;

  messageCount--;
  trimTimes++;
}

Thank you to everyone that helped. The function works properly now. To those asking why I was trying to print out an array I just freed, that was just there to show that the problem occurred after strcpy and rule out any other code that came after it.

The final code is here, in case it proves useful to anyone who comes across a similar problem:

void trimMessage()
{
  int trimIndex;
  size_t msgSize = strlen(currMessage);

  char *newline = strchr(currMessage, '\n'); 
  if (!newline) return;
  trimIndex = newline - currMessage;

  // e.g.: "\fProgram started\r\nHow are you?\r".
  char *trimMessage = malloc(msgSize - trimIndex + 1);

  trimMessage[0] = '\f';
  strcpy(&trimMessage[1], &currMessage[trimIndex+1]);

  trimMessage[msgSize - trimIndex] = '\0';

  // Frees up the memory allocated to currMessage from last iteration
  // before assigning new memory.
  free(currMessage);
  currMessage = malloc(msgSize - trimIndex + 1);

  for(int i=0; i < msgSize - trimIndex; i++)
  {
    currMessage[i] = trimMessage[i];
  }

  currMessage[msgSize - trimIndex] = '\0';
  free(trimMessage);

  messageCount--;
}

Solution

  • free can and will crash if the heap is corrupt or if you pass it an invalid pointer.

    Looking at that, I think your first malloc is a couple of bytes short. You need to reserve a byte for the null terminator and also you're copying into offset 1, so you need to reserve another byte for that. So what is going to happen is that your copy will overwrite information at the start of the next block of heap (often used for length of next block of heap and an indication as to whether or not it is used, but this depends on your RTL).

    When you next do a free, it may well try to coalesce any free blocks. Unfortunately, you've corrupted the next blocks header, at which point it will go a bit insane.