Thread: Recursion with pointer and memory leak

  1. #1
    Registered User
    Join Date
    May 2012
    Posts
    4

    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;
    }

  2. #2
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    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

  3. #3
    Registered User
    Join Date
    May 2012
    Posts
    4
    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?

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    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[].
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    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

  6. #6
    Registered User
    Join Date
    May 2012
    Posts
    4
    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.

  7. #7
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    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.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  8. #8
    Registered User
    Join Date
    May 2012
    Posts
    4
    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]);

  9. #9
    Registered User
    Join Date
    May 2010
    Posts
    4,633
    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.

    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

  10. #10
    Registered User
    Join Date
    Oct 2006
    Posts
    3,445
    Quote Originally Posted by Dang Scholar View Post
    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.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Possible memory leak?
    By 127.0.0.1 in forum C Programming
    Replies: 21
    Last Post: 05-02-2011, 04:53 PM
  2. Replies: 2
    Last Post: 09-28-2006, 01:06 PM
  3. Memory Leak?
    By BigDaddyDrew in forum C++ Programming
    Replies: 11
    Last Post: 12-09-2002, 04:28 PM
  4. Memory Leak or not?
    By Eber Kain in forum C++ Programming
    Replies: 4
    Last Post: 11-21-2001, 01:05 PM
  5. memory leak
    By gatli in forum C++ Programming
    Replies: 4
    Last Post: 09-19-2001, 09:07 AM

Tags for this Thread