Recursion with pointer and memory leak

I have tried to write a simple program that converts numbers to words (i.e it converts 123 to one hundred twenty three). The code compiles completely. The code uses pointer and recursion, which I always find tricky, in terms of memory management, in c++. I am using a point to character (word) in function (convert_number) and returning the same value from that function, but I am not sure how to free the memory that it is using after the function returns the value. Following is the code,
It's much app reciated if anyone can point out other memory leaks in the program.

Code:

---

#include <iostream>
#include <string>
using namespace std;
char *convert_number(int, int);


const char *tens[]={"","ten", "twenty", "thirty", "forty", "fifty", "sixty", "seventy", "eighty", "ninety"};
const char *words[]={"zero","one", "two", "three", "four", "five", "six", "seven", "eight", "nine","ten","eleven","twelve","thirteen","fourteen","fifteen","sixteen","seventeen", "eighteen","ninteen"};
const char *place[]={"","thouands","million","billion","trillion"};
   
   
int main(int argc, char **argv)
{
    int number,conv_num,places;
    places=1;
    char *string;
    cout<<"Enter a number:";
    cin>>number;
    string = convert_number(number,0);
    cout<<"The word is :"<<string<<endl;
}


char *convert_number(int number,int places)
{
    int divisor;
    char *word;
    int conv_num;
    char *temp_string;
    word = new char[100];
    divisor=10;
    if (number>=1000)
    {   
        conv_num = number % 1000;
        number = (number-conv_num)/1000;
        places++;
        temp_string = convert_number(number,places);   
        word = strcat(word, temp_string);
        word = strcat(word, place[places]);
        word =strcat(word," ");
    }
    else
    {
        conv_num = number;
    }
    if (conv_num>=100)
    {
        word =strcat(word,words[conv_num/100]);
        word =strcat(word," hundred ");
        conv_num=conv_num%100;
    }
   
    if(conv_num >=20)
    {
    word=strcat(word,tens[conv_num/10]);
    word =strcat(word," ");
    if(conv_num%divisor>=1)
    {
        word=strcat(word,words[conv_num%divisor]);   
        word =strcat(word," ");
      }
    }
   
    if(conv_num<20)
    {
        word=strcat(word,words[conv_num]);
        word =strcat(word," ");
    }   
               
    return word;
}

---

Since this is a C++ program why are you using C-strings instead of std::string? You wouldn't need the dynamic memory if you used a std::string and therefore you wouldn't need to worry about deleting this memory. As for the memory leak it looks like you should be deleting temp_string in your if statement after the strcat() call and possibly delete string (a very bad name for a variable in a C++ program since you have included the <string> header file) after it's use in main as well. Also you need to include the <cstring> header file for the C-string functions like strcat().

Jim

Thanks Jim, using std::string is a nice option and deleting temp_string sounds good. But still 'word' is point to some memory which holds 100 bytes. I return 'word' from this function, and since the stack is lost after function returns,I do not know how to free the memory it is using. Is there a way we can free this memory?

Quote:

---

Originally Posted by Dang Scholar

But still 'word' is point to some memory which holds 100 bytes. I return 'word' from this function, and since the stack is lost after function returns,I do not know how to free the memory it is using. Is there a way we can free this memory?

---

The memory allocated for what word points to is on the heap (or should I say free store?), not the stack. You should delete[] what you new[].

But the only place you are recursively calling this function is in your one if statement, if you delete the temp_string in that if statement you should be set to go, except for when the program actually returns to your calling function. Which is why you delete this memory after the variable is no longer needed in main.

Code:

---

    if (number>=1000)
    { 
        conv_num = number % 1000;
        number = (number-conv_num)/1000;
        places++;
        temp_string = convert_number(number,places); 
        word = strcat(word, temp_string); // Delete the memory after this call, temp_string is not used in the rest of the function.
        word = strcat(word, place[places]);
        word =strcat(word," ");
    }

---

But as I said using std::string is much cleaner, no need for all the strcat() calls, dynamic memory, etc.


Jim

Quote:

---

The memory allocated for what word points to is on the heap (or should I say free store?), not the stack. You should delete[] what you new[].

---

Its true that you I should delete[] what I new[] but, my question is where?
If I do delete[] word before it is returned then its not useful anymore.
If I delete after its returned its different memory location. Since I have recursive function which creates its own copy of word when the function is called recursively.

Quote:

---

Originally Posted by Dang Scholar

If I delete after its returned its different memory location.

---

No, it is the same memory location. Refer to jimblumberg's post #5 for one place where you should use delete[], and your main function for another place. As mentioned, just using std::string will handle this for you.

Thanks you guys, I think that solved my problem. I used following code,
I did not know that i could do delete[] temp_string even when it was declared as char* temp_string.

Code:

---

cout<<"Enter a number:";
cin>>number;
string = convert_number(number,0);
delete[] string;
cout<<"The word is :"<<string<<endl;

---

:
:
:

Code:

---

temp_string = convert_number(number,places);       
word = strcat(word, temp_string);
delete[] temp_string;
word = strcat(word, place[places]);

---

In your main function you should delete[] the memory after you are through with it, not before. Therefore you should delete[] the memory after the cout.

And also since you included the <string> header, are using the using namespace std clause you should not use "string" for a variable name, string is a type.

Quote:

---

I did not know that i could do delete[] temp_string even when it was declared as char* temp_string.

---

When you returned from that function you assigned the address of your "new" memory to temp_string, before you returned this pointer from the function temp_string was an uninitialized variable. After the function returns it now has the address of your "new" memory which is why you can safely delete the memory.

Jim

Quote:

---

Originally Posted by Dang Scholar

Code:

---

string = convert_number(number,0);
delete[] string;
cout<<"The word is :"<<string<<endl;

---

---

this code is a bad idea. call it something other than "string." you're including <string> and using namespace std, so std::string becomes global as string, and naming a variable after a type in the global namespace is going to come back to bite you at some point. also, why are you still using manual memory management instead of std::string? your program would be about half as long if you used std::string. not to mention, with std::string you NEVER have to worry about freeing memory. you won't have memory leaks, you won't have double deletions.