2

My code is getting the following error:

free(): invalid next size (fast)

My code:

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


  int main()
  {  
char *temp_str;
char *pos, *pos2;
char *key = (char*)malloc(20);
char *data = (char*)malloc(20);
const char newline = '\n';

char *unparsed_data = "NEW_DATA_PACKET\n\
  Data Set 16-byte Universal Key\n\
  Unix Time Stamp (microsecs): 1319639501097446\n\
  Frame Number: 0\n\
  Version: 3\n\
  Platform Yaw Angle (deg): 15.22428\n\
  Platform Pitch Angle (deg): 1.78528\n\
  Platform Roll Angle (deg): 2.004111\n\
  Image Source Source: test\n\
  Image Coordinate System: Testing XXXXX\n\
  Sample Latitude (deg): 17.306791\n\
  Sample Longitude (deg): -60.26209\n\
  Sample True Altitude (m): 7.623407\n\
  Sample Horizontal FoV (deg): 0.1821277\n\
  Sample Vertical FoV (deg): 15.1879\n\
  Sample Rel. Azimuth Angle (deg): 291.70295\n\
  Sample Rel. Elevation Angle (deg): -5.941163\n\
  Sample Rel. Roll Angle (deg): 3.10959\n\
  Checksum: 4659\n";

temp_str = (char*)malloc(strlen(unparsed_data));

printf("\nThe original string is: \n%s\n",unparsed_data);

//Ignore the first two lines
pos = strchr(unparsed_data, newline);
strcpy(temp_str, pos+1);
pos = strchr(temp_str, newline);
strcpy(temp_str, pos+1);
printf("STARTING THE PARSER\n\n");
while(strlen(temp_str) > 2)
{
  printf("\tstarting loop\n");
  //Getting the position of the colon and newline character
  pos = strchr(temp_str, ':'); // ':' divides the name from the value
  pos2 = strchr(temp_str, '\n'); //end of the line
  realloc(key, (pos-temp_str-1)); //allocate enough memory
  realloc(data, (pos2-pos)-1);

  printf("After realloc \n");

  //copying into key and data
  strncpy(key, temp_str, (pos-temp_str));
  strncpy(data, pos + 2, (pos2-pos)-2);

  //Append null terminator
  strcpy(key+(pos-temp_str-1)+1, "");
  strcpy(data+(pos2-pos)-2, "");


  printf("%s = ",key);
  printf("%s\n",data);

  assign_value(key,data, &gvals);

  printf("The value has been assigned\n");
  strcpy(temp_str, pos2+1);

  printf("Freeing key and data\n\n");

  free(key);
  free(data);

}

return 0;
  }

I am also getting some garbage when it is printing

STARTING THE PARSER

    starting loop
After realloc 
Unix Time Stamp (m13196395 = 13196395
The value has been assigned
Freeing key and data

*** glibc detected *** ./memory_alloc: free(): invalid next size (fast): 0x098cd008 ***

Finally, when I reduce the length of the lines of the string, the code works perfectly. It works fine with the following string:

char *unparsed_data = "NEW_DATA_PACKET\n\
  Data Set 16-byte Universal Key\n\
  Time Stamp: 1319639501097446\n\
  Frame Number: 0\n\
  Version: 3\n\
  Yaw Angle: 15.22428\n\
  Pitch Angle: 0.78528\n\
  Roll Angle: 2.004111\n\
  Source: test\n\
  Coor System: Test XXXXX\n\
  Latitude: 27.306791\n\
  Longitude: -60.26209\n\
  True Altitude: 7.623407\n\
  Hor FoV: 0.1821277\n\
  Ver FoV: 15.1879\n\
  Azimuth: 291.702954\n\
  Elevation: -0.433563\n\
  Roll: 0.79659\n\
  Checksum: 2659\n";

** *Solution:* **

int main()
      {  
    char *temp_str;
    char *pos, *pos2;
    char *key = (char*)malloc(20);
    char *data = (char*)malloc(20);
    const char newline = '\n';

    char *unparsed_data = "NEW_DATA_PACKET\n\
The UAS Datalink Local Data Set 16-byte Universal Key\n\
Time Stamp: 1319639501097446\n\
Frame Number: 0\n\
Version: 3\n\
Yaw Angle: 15.22428\n\
Pitch Angle: 0.78528\n\
Roll Angle: 2.004111\n\
Source: test\n\
Coor System: Test XXXXX\n\
Latitude: 27.306791\n\
Longitude: -60.26209\n\
True Altitude: 7.623407\n\
Hor FoV: 0.1821277\n\
Ver FoV: 15.1879\n\
Azimuth: 291.702954\n\
Elevation: -0.433563\n\
Roll: 0.79659\n\
Checksum: 2659\n";


    temp_str = (char*)malloc(strlen(unparsed_data));

    //Ignore the first two lines
    pos = strchr(unparsed_data, newline);
    strcpy(temp_str, pos+1);
    pos = strchr(temp_str, newline);
    strcpy(temp_str, pos+1);
    printf("STARTING THE PARSER\n\n");
    while(strlen(temp_str) > 2)
    {
      printf("\tstarting loop\n");
      //Getting the position of the colon and newline character
      pos = strchr(temp_str, ':'); // ':' divides the name from the value
      pos2 = strchr(temp_str, '\n'); //end of the line
      char *new_key = (char*)realloc(key, (pos-temp_str+1)); //allocate enough memory
      char *new_data = (char*)realloc(data, (pos2-pos+1));


      if(new_key)
        key = new_key;
      if(new_data)
        data = new_data;

      printf("After realloc \n");

      //copying into key and data
      strncpy(key, temp_str, (pos-temp_str));
      strncpy(data, pos + 2, (pos2-pos)-2);

      //Append null terminator
      memset(key + (pos-temp_str) , '\0', 1);
      memset(data + (pos2 - pos), '\0', 1);


      printf("%s = ",key);
      printf("%s\n",data);

      assign_value(key,data, &gvals);

      printf("The value has been assigned\n");
      strcpy(temp_str, pos2+1);

      printf("Freeing key and data\n\n");

      free(key); key= NULL;
      free(data); data = NULL;

    }

    return 0;
      }
dead_jake
  • 523
  • 2
  • 12
  • 30

2 Answers2

4

Here is the mistake:

realloc(key, (pos-temp_str-1)); //allocate enough memory
realloc(data, (pos2-pos)-1);

realloc() returns the address of the new buffer (which may be different), with the buffer that was passed in potentially being deallocated. The old buffers are then passed to free(), but they may have already been deallocated. You need to save the return value from realloc():

char* new_key = realloc(key, ...); /* Don't assign key = realloc(key, ...);
                                      Since if realloc() fails it returns
                                      NULL, but leaves the original
                                      buffer unmodified. This will
                                      result in a memory leak. */
if (new_key)
{
    key = new_key;
}

free(key);
key = NULL; /* Be sure assign key to NULL, otherwise dangling pointer will
               be passed to realloc() on next iteration. */

Also, see Do I cast the result of malloc?

Tomer
  • 531
  • 7
  • 19
hmjd
  • 120,187
  • 20
  • 207
  • 252
  • Great! It worked, thanks. I also used part of @wallyk solution, and used memset to append the null terminator. – dead_jake Jun 25 '12 at 16:57
  • I also read about casting realloc, but I was getting `invalid conversion from ‘void*’ to ‘char*’ [-fpermissive]` – dead_jake Jun 25 '12 at 17:18
1

There are subtle problems in this code:

realloc(key, (pos-temp_str-1)); //allocate enough memory
realloc(data, (pos2-pos)-1);

  ...
strncpy(key, temp_str, (pos-temp_str));
strncpy(data, pos + 2, (pos2-pos)-2);

The reallocated length is n-1, but it copies n characters to it. This would be better:

key = realloc(key, pos - temp_str + 1);
memset (key, 0, pos - temp_str + 1);  // make sure terminating NUL present

data = realloc(data, pos2 - pos + 1);
memset (data, 0, pos2 - pos + 1);
   ...
strncpy(key, temp_str, pos - temp_str);
strncpy(data, pos + 2, pos2 - pos - 2);
wallyk
  • 56,922
  • 16
  • 83
  • 148