C++ can't delete char*, keeps corrupting heap

Question

Language: C++, Compiler: MSVS (/Za) and g++ (yes, it must work on both), Level: beginner

I'm attempting to delete the data from a char* so that I can reallocate it and continue my program. I create a new char*, assign the value of the command line parameter to it, do some validation, then if the validation fails it should deallocate the char* and let me assign new data to the var however I'm getting a "Heap corruption detected" error in visual studio. I'm curious about both the fix for my current code and any other ways this could be done more clearly/concisely.

In main():

//...
//make non constant versions to be messed with
char* ba = new char[ strlen(argv[4]) + 1 ];
char* num = new char[ strlen(argv[2]) + 1 ];

//make non constant versions of commandline stuff
nonConstant( argv[4], argv[2], ba, num );

//do the conversion and printing
convert( ba, num );
//...

convert does this:

//...
if( error ) {
    getNew(ba, num, &error);
}
//...

and here is getNew:

void getNew( char* ba, char* num, bool error ) {

//if there's an error in their numbers let them input new stuff and check to see if that's valid
while( error ) {
    //tell the user that the input was bad an prompt for more, use getline because strings are weird
    //cin stuff here (this part works, no problems)

    //free up base and num so I can reassign them
    delete[] ba; //<--this line fails
    delete[] num;

    //set lengths = to lengths of input + 1 for \0
    ba = new char[ inputBa.length() + 1 ];
    num = new char[ inputNum.length() + 1 ];

    //do the assignment of new input back to base and num
    inputBa.copy( ba, inputBa.length(), 0 );
    inputNum.copy( num, inputNum.length(), 0 );

    //ensure that the input was good this time
    validateInput( ba, num, error );
}

Answer 1

Pass parameters by char *& will fix the problem.

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

void reassgnReference( char * &ba, char * &num );
void reassgnPointerToPointer( char ** ba, char ** num );

int main( int argc, char ** argv )
{
    if ( argc > 4 )
    {
        char * ba = new char[strlen( argv[ 4 ] ) + 1];
        char * num = new char[strlen( argv[ 2 ] ) + 1];

        strncpy( ba, argv[ 4 ], strlen( argv[ 4 ] ) );
        strncpy( num, argv[ 2 ], strlen( argv[ 2 ] ) );

        printf( "In the beginning, ba = %s, num = %s\n", ba, num );

        reassgnReference( ba, num );
        printf( "reassgnReference(), ba = %s, num = %s\n", ba, num );

        reassgnPointerToPointer( &ba, &num );
        printf( "reassgnPointerToPointer(), ba = %s, num = %s\n", ba, num );
}
else
{
    printf( "%s Expects at least 4 arguments\n", argv[ 0 ] );
}

return( 0 );
}

void reassgnReference( char *& ba, char *& num )
{
    delete[] ba;
    delete[] num;

    char const * const newBa = "ba from reference";
    char const * const newNum = "num from reference";

    ba = new char[strlen( newBa ) + 1];
    num = new char[strlen( newNum ) + 1];

    strncpy( ba, newBa, strlen( newBa ) );
    strncpy( num, newNum, strlen( newNum ) );
}

void reassgnPointerToPointer( char ** ba, char ** num )
{ 
    delete[] *ba;
    delete[] *num;

    char const * const newBa = "ba from pointer to pointer";
    char const * const newNum = "num from pointer to pointer";

    *ba = new char[strlen( newBa ) + 1];
    *num = new char[strlen( newNum ) + 1];

    strncpy( *ba, newBa, strlen( newBa ) );
    strncpy( *num, newNum, strlen( newNum ) );
}

Answer 2

If looks like your getNew function can reallocate the memory local ba and num are pointing to, i.e delete[] it and new[] it again.

But what about ba and num in the caller, in convert? Also, what about ba and num in main? They are passed to getNew by value, which means that they remain unchanged. They continue to point to their original (now dead) memory locations. I would guess that you continue to use those now-meaningless ba and num values in convert and in main until they cause the crash. For example, if you call getNew with those old pointer values again, getNew will likely fail at delete[].

Also, it is not immediately clear what error is in convert. Why are you passing it as &error (likely a pointer) when getNew actually expects a bool argument? It will compile, but it looks like a problem anyway.

Answer 3

Stop! Switch to std::string, immediately.

This will both fix your code, and be more clear/concise, as requested.