New declaration is containing garbage value and Heap corruption while using delete

Question

I'm trying to read an ANSI formatted file and converting this to binary.I'm declaring two dynamic memory allocation like this: char* binary_reverse = new char; and char * binary = new char;

While debugging I see this(binary) contains too many garbage values. Why is it so?

I'm deleting these like: delete binary_reverse; delete binary; However, during delete its giving me error:

'ASCIItoBinary.exe': Loaded 'D:\TryingBest\Reactice\ASCIItoBinary\Debug\ASCIItoBinary.exe', Symbols loaded. 'ASCIItoBinary.exe': Loaded 'C:\Windows\SysWOW64\ntdll.dll', Cannot find or open the PDB file 'ASCIItoBinary.exe': Loaded 'C:\Windows\SysWOW64\kernel32.dll', Cannot find or open the PDB file 'ASCIItoBinary.exe': Loaded 'C:\Windows\SysWOW64\KernelBase.dll', Cannot find or open the PDB file 'ASCIItoBinary.exe': Loaded 'C:\Windows\SysWOW64\msvcr100d.dll', Symbols loaded. HEAP[ASCIItoBinary.exe]: Heap block at 00241ED0 modified at 00241EFD past requested size of 25 Windows has triggered a breakpoint in ASCIItoBinary.exe.

Here is how I'm doing code:

#include <cstring>

void AtoB(char * input)
{
    unsigned int ascii; //used to store ASCII number of a character
    unsigned int length = strlen(input);
    //cout << " ";
    for (int x = 0; x < length; x++) //repeat until the input is read
    {
        ascii = input[x];
        char* binary_reverse = new char;       //dynamic memory allocation
        char * binary = new char;
        //char binary[8];
        int y = 0;
        while (ascii != 1)
        {
            if (ascii % 2 == 0)    //if ascii is divisible by 2
            {
                binary_reverse[y] = '0';   //then put a zero
            }
            else if (ascii % 2 == 1)    //if it isnt divisible by 2
            {
                binary_reverse[y] = '1';   //then put a 1
            }
            ascii /= 2;    //find the quotient of ascii / 2
            y++;    //add 1 to y for next loop
        }
        if (ascii == 1)    //when ascii is 1, we have to add 1 to the beginning
        {
            binary_reverse[y] = '1';
            y++;
        }

        if (y < 8)  //add zeros to the end of string if not 8 characters (1 byte)
        {
            for (; y < 8; y++)  //add until binary_reverse[7] (8th element)
            {
                binary_reverse[y] = '0';
            }
        }

        for (int z = 0; z < 8; z++)  //our array is reversed. put the numbers in the rigth order (last comes first)
        {
            binary[z] = binary_reverse[7 - z];
        }
        //printf("the Binary is %s",binary);
        //cout << binary;   //display the 8 digit binary number

        delete binary_reverse;     //free the memory created by dynamic mem. allocation
        delete binary;
    }
}

I want the exact binary values in "binary". NOT the binary values along with garbage?How to eliminate the garbage values? How to avoid the heap corruption?

Answer 1

The problem is that you are allocating just 1 character with the new char commands. You want to allocate more, using new char[9]. Since you are printing out a maximum of 8 bits, you need one extra char for the null terminator. Be sure to set binary_reverse[y]=0 at the end of the string.

And then delete[] instead of delete.

But that said, you should be using std::string or std::vector instead...

Answer 2

Turns out there are a bunch of things wrong in here almost all of them stem from not terminating the output string, and then heading offf in the wrong direction looking for a fix.

I'm going to ignore the error in

char* binary_reverse = new char;

other than to say OP needed more storage.

char* binary_reverse = new char[8];

The correct approach is to go back to the temporary allocation it looks like OP started with and add an extra byte to contain the string's null terminator. And then use that space for a null terminator.

Without the null terminator, you don't have a string. You have a binary blob. The printing routines, all c-style string routines, count on that terminator being there. Without it they don't know where the string ends and head off into the wild blue yonder looking for it. Often bad stuff happens. Or maybe it doesn't. What happens when you walk outside an array is undefined. Maybe it does what you want. Maybe it doesn't. No way to be sure. Nuking the site from orbit doesn't even work in this case.

So allocate temporary storage:

char binary_reverse[8]; // not using this one like a string so we don't need a terminator
char binary[9]; // printing this one. Need a terminator to know when to stop printing.

Later, after binary_reverse is constructed and then transferred to binary, binary needs to be terminated to become a string rather than just another anonymous binary blob.

binary[8] = '\0';

Now it can be printed.

Recommendations:

Visual Studio has an awesome debugger. Familiarize yourself with it. Save you much time it will.

If OP hadn't commented out the print statements odds are good someone would have spotted the main bug last night. Minimizing code is good, but OP removed the visible manifestation of the bug.

This code can be dramatically simplified. You know you want 8 bits because you're working in ascii (Well, actually ascii is 7 bits, but it's rare to see anything but 8 bit anymore). Turn the while (ascii != 1) into for (int count = 0; count < 8; count++) and test all 8 bits in the character. Save you a couple loops later because now you ALWAYS get 8 bits.