Question

I am trying to write a program that reverses a users input string. if they enter "book" it should return "koob". I have to call the function reverseit(char *inputString, char *outputString) and write my algorithm in that.

I have everything working but it returns a malloc error at the end

:malloc: *** error for object 0x7fff5c8a5bc0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

The code i wrote:

void reverseit(char *inputString, char* outputString);

int main()
{
    char input[100];
    cin.getline(input, 10);
    int lenn = strlen(input); 
    char* usrinput = new char[lenn+1];
    usrinput = input;
    int len = strlen(usrinput);
    char* inputString = new char[len+1];
    inputString = usrinput;
    char* outputString = new char[len+1];
    reverseit(inputString, outputString);
    for (int o = 0; o < len; o++)
    {
        cout << outputString[o];
    }
    cout << endl;
    delete [] usrinput;
    delete [] inputString;
    delete [] outputString;
    return 0;

}

void reverseit(char *inputString, char *outputString)
{
    size_t length = strlen(inputString);
    char temp;
    int k = 0;
    for(int i = length - 1; i >= 0 ; i--)
    {
        temp = inputString[i];
        outputString[k] = temp;
        k++;
    }
}

There are several restrictions that are really making this more difficult than it actually is, because I am not allowed to call any other variables into reverseit except the two char pointers.

My specific question is if I am deleting all my dynamic arrays how are the memory leaks still happening, could the leak be happening because I am not entering the correct size for each array? Thank you

Was it helpful?

Solution

usrinput = input;
inputString = usrinput;

To copy strings, you need to copy every char, not just reassign pointers. Using = throws away the dynamically allocated memory—that's the memory leak—and causes all the pointers to point at the same location in memory. That means both usrinput and inputString end up pointing to input, and consequently the delete[] calls at the end fail since input wasn't allocated with new.

strcpy(userinput, input);
strcpy(inputString, usrinput);

OTHER TIPS

The problem are these lines:

usrinput = input;
/* ... */
inputString = usrinput;

This is not the correct way to copy a string. Use strcpy() instead:

strcpy(usrinput, input);

Since you changed the value of usrinput, you were calling delete[] on a different pointer (specifically, the stack-allocated array input) than what you allocated with new[].

You're freeing an array that's on the stack:

char input[100];      
...
usrinput = input;
...
delete [] usrinput;

I'm not entirely sure what you're trying to accomplish here, but it certainly isn't what you're doing. If you want the allocated memory to hold the values of the string on the stack, you have to do something like:

my_buffer[100];
set_my_buffer(my_buffer);
char *my_allocated_buffer = new char [100];
strncpy(my_allocated_buffer, my_buffer, 100);

Your setting of input into usrinput is incorrect. input is of type char[] and usrinput is of type char *. When you do this, it is re-pointing usrinput to a different memory location as was assigned by the new()[] operator.
When you now try to delete[], the variable now points to your statically assigned char[] type, not the one you dynamically allocated.

int main(){

    char input[100];                   // Statically Allocated
    cin.getline(input, 10);
    int lenn = strlen(input); 
    char* usrinput = new char[lenn+1]; // DYNAMICALLY allocated
    usrinput = input;                  // setting statically allocated into variable that
                                       // previously held address of dynamically allocated
                                       // here is where your dynamic pointer gets LOST

    int len = strlen(usrinput);
    char* inputString = new char[len+1];  // Another Dynamic Allocation
    inputString = usrinput;               // This WOULD be OK except that now you are losing the
                                          // where inputString used to point to which was 
                                          // dynamically allocated

    char* outputString = new char[len+1];
    reverseit(inputString, outputString);
    for (int o = 0; o < len; o++)
    {
        cout << outputString[o];
    }
    cout << endl;
    delete [] usrinput;                    // usrinput now doesn't point to a dynamically
                                           // allocated space in memory, neither does inputString
    delete [] inputString;
    delete [] outputString;
    return 0;
}

You allocated new memory locations and but you reassigned the pointers back to the original location.

Example:

int * pointer1 = new int[1];
pointer1[0] = 10;
int * pointer2 = new int[1];
pointer2[0] = 20;
cout << pointer2[0]; // 20
pointer2 = pointer1; 
cout << pointer2[0]; // 10
cout << pointer1[0]; // 10
delete [] pointer2;
cout << pointer1[0]; // Oh no! Garbage Value! But we didn't delete pointer1 did we?
delete [] pointer1; // FATAL ERROR!

The problem with this code is that when pointer2 = pointer1, it merely gave pointer2 the address of pointer1. It did not copy any values over. This also means that the original space allocated for pointer2 is forever lost in limbo. Never freed. This is what caused the problem in your code on lines:

char* usrinput = new char[lenn+1];
usrinput = input;

You want to copy all the values over. Use strcpy()

strcpy(usrinput, input)

The strings contained in a character array cannot be copied by assigning one pointer to another. You need to use a function like strcpy which searches the memory pointed to by the pointer until it reaches a null-terminator ('\0') and copies the content to a memory pointed to by the destination.

Example:

// Initialize an array of characters which contain the string "Foo bar\0";
char input[] = "Foo bar";
// Allocate a character array on the heap and assign it to the pointer pDest
char *pDest = new char[100];
// Reassign the pointer to the first element of input
pDest = input;
delete [] pDest; // Oops: Deleting the first element of input!

Instead, you should do this:

char input[] = "Foo bar";
char *pDest = new char[100];
// Copy all chars in the memory at input to the memory pointed to by pDest
strcpy(pDest, input);
delete [] pDest; // pDest still points to the dynamic memory, everything's fine

Please note that using strcpy can be dangerous, it simply copies all content it can find until it reaches the terminating null to the destination, it doesn't (cannot!) check if the destination is actually large enough. In your example you already avoid this by using strlen, which is fine. It's just the assignment which doesn't work the way you think.

Apart from this error, there are other things which can be improved in your code:

  • There is no need to make any copies at all, use getline to get the user input, allocate a new array with strlen + 1 of this user input and you're done, you can pass both of these variables to reverseit (remember to add a '\0' delimiter at the end of reverseit.
  • The temporary char in reverseit is unnecessary, just assign inputString[i] directly to outputString[k]
  • You can put both i and k in the for declaration, no need to handle k differently
  • cout can handle strings in the form of char*, you don't need a loop the write the result to the console, just write cout << outputString

Handling strings in C++ is made much easier with the std::string class, however I guess this is some sort of homework and you have to stick to the basics.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top