C Memory Allocation Error

Question

Hi I am working on a project, where I need to Pack files into a PFS Image. I am writing an application using the ANSI C language. I am getting each files Hexdump and other attributes and storing inside variables.

Once all Information about the Files being packed is gathered, I need to create an output file with each file's infromation.

As I am doing this, I am having trouble with memory allocation. The Code outputting the error is as follows.

for (Counter = 0; Counter < PackingCount; Counter ++)
{
    PFSEntry Packed;

    Packed.HexEquivalent = DumpHex(FileNames[Counter]);

    strncpy(Packed.Filename, FileNames[Counter], NAME_BLOCK);

    Packed.Offset = OffsetCounter;

    OffsetCounter += FileSize;

    Packed.FileSize = FileSize;

    Packed.Timestamp = 2999606509; // For the Sake of Diffing

    Packer[Counter] = Packed;


}

The structure which the loop above fills is shown below

typedef struct
{
 char Filename [NAME_BLOCK];
 u_int32_t Timestamp;
 u_int32_t Offset;
 u_int32_t FileSize;
 char * HexEquivalent;
} PFSEntry;

and the DumpHex Function is as follows:

char * DumpHex(char * FileName)
{
   FILE *  File = FileOpener(FileName, "rb");

   printf("%s is of Size %ld\r\n\r\n", FileName, FileSize);

   fseek(File, 0L, SEEK_END);

   FileSize = ftell(File);

   fseek(File, 0L, SEEK_SET);

   char * HexArray = malloc(FileSize);

   unsigned char Character;

   int Counter = 0;

   while (Counter < FileSize)
   {
       Character = fgetc(File);     
       sprintf(HexArray  + Counter, "%c", Character);       
       Counter++;
   }

   return HexArray;
}

The Function DumpHex, which returns the hex output of the given file is outputting the following error.

a.out: malloc.c:2369: sysmalloc: Assertion `(old_top == (((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) && old_size == 0) || ((unsigned long) (old_size)

= (unsigned long)((((__builtin_offsetof (struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * (sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end & pagemask) == 0)' failed. Aborted (core dumped)

Below are some Debug information added to the application that may help in the finding solution.

Total Files to Pack 38 
Size of Packed Structure 80
Packing File 0 of size 9319 Bytes
Packing File 1 of size 1459 Bytes
Packing File 2 of size 844 Bytes
Packing File 3 of size 4396 Bytes
Packing File 4 of size 270250 Bytes
Packing File 5 of size 656800 Bytes
Packing File 6 of size 0 Bytes
Packing File 7 of size 322744 Bytes
Packing File 8 of size 1278114 Bytes
Packing File 9 of size 12473 Bytes
Packing File 10 of size 13791 Bytes
Packing File 11 of size 14158899 Bytes
Packing File 12 of size 343051 Bytes
Packing File 13 of size 599051 Bytes
Packing File 14 of size 505867 Bytes
Packing File 15 of size 10138349 Bytes
Packing File 16 of size 17481 Bytes
Packing File 17 of size 4900 Bytes
Packing File 18 of size 9000 Bytes
Packing File 19 of size 343 Bytes
Packing File 20 of size 6888 Bytes
Packing File 21 of size 13992 Bytes
Packing File 22 of size 916222 Bytes
Packing File 23 of size 2048 Bytes
Packing File 24 of size 7776 Bytes
Packing File 25 of size 13884 Bytes
Packing File 26 of size 10787 Bytes
Packing File 27 of size 12747 Bytes

a.out: malloc.c:2369: sysmalloc: Assertion `(old_top == (((mbinptr) (((char *) 
&((av)->bins[((1) - 1) * 2])) - __builtin_offsetof (struct malloc_chunk, fd)))) &&
old_size == 0) || ((unsigned long) (old_size) >= (unsigned long)((((__builtin_offsetof
(struct malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 *
(sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end &
pagemask) == 0)' failed.

Aborted (core dumped)

I am new to the language, and I don't quite understand the concept of memory allocation and the free method.

Answer 1

From the code shown here, and given the error, it looks like you have an out of bounds array access somewhere that is corrupting malloc's own data structures.

The fact that it works with some files is sheer luck, that's the problem with undefined behaviour - behaving like expected is a form of undefined behaviour, this is what makes bugs like these hard to track.

From what I can see here, this is wrong:

   while (Counter < FileSize)
   {
       Character = fgetc(File);     
       sprintf(HexArray  + Counter, "%c", Character);       
       Counter++;
   }

HexArray is a dynamically allocated array of FileSize bytes. However, note that sprintf() always terminates the output string with a null-byte. Thus, for each iteration, HexArray[Counter] is set to Character, and HexArray[Counter+1] is set to a null byte. There's no harm in this except in the last iteration. When Counter is FileSize-1 (the last iteration), sprintf() will be writing a null byte into HexArray[FileSize] - out of bounds access. This is undefined behaviour and will most likely corrupt malloc data structures, thus yielding the cryptic errors later in the program.

If all you want to do is to write a character to each position in HexArray, you can use the much more efficient and less error-prone form:

   while (Counter < FileSize)
   {
       Character = fgetc(File);
       HexArray[Counter++] = Character;
   }

Also, since Character is unsigned char, you should change HexArray from char * to unsigned char *.

Consider also what happens with huge files (if your program is supposed to be invoked with those). Memory exhaustion is a reality, especially if you're developing for an embedded system (which seems to be the case).