Thread: Possible memory leak.

  1. #1
    Novice
    Join Date
    Jul 2009
    Posts
    568

    Possible memory leak.

    I'm doing an exercise from C++ Primer book, and I suspect that this bit of code I've written is leaking memory.

    Code:
    struct Stringy {
        char* str;
        int ct; // lenght w/o '\0';
    };
    
    void set(Stringy& st, const char s[]);
    
    // main()
    
    void set(Stringy& st, const char s[]) {
        using namespace std;
        
        int len = strlen(s);
        char* t = new char[len + 1];
        
        st.ct = len;    
        st.str = t;
        strcpy(st.str, s);
    }
    Now, the way I understand this is that every time set() is called new block of memory is allocated for t and then st.str is set to point to this block. My concern is that the block st.str previously pointed to is, to the best of my understanding, never released.

    So my questions are (1) do I understand the situation correctly and if yes (2) what can be done to remedy it?

  2. #2
    C++11 User Tux0r's Avatar
    Join Date
    Nov 2008
    Location
    Sweden
    Posts
    135
    Quote Originally Posted by msh View Post
    I'm doing an exercise from C++ Primer book, and I suspect that this bit of code I've written is leaking memory.

    Code:
    struct Stringy {
        char* str;
        int ct; // lenght w/o '\0';
    };
    
    void set(Stringy& st, const char s[]);
    
    // main()
    
    void set(Stringy& st, const char s[]) {
        using namespace std;
        
        int len = strlen(s);
        char* t = new char[len + 1];
        
        st.ct = len;    
        st.str = t;
        strcpy(st.str, s);
    }
    Now, the way I understand this is that every time set() is called new block of memory is allocated for t and then st.str is set to point to this block. My concern is that the block st.str previously pointed to is, to the best of my understanding, never released.

    So my questions are (1) do I understand the situation correctly and if yes (2) what can be done to remedy it?
    in your set function, use delete[] st.str;

    Code:
    #include <iostream>
    #include <cstring>
    
    struct Stringy {
        char* str;
        int ct; // lenght w/o '\0'
    };
    
    void set(Stringy& st, const char s[]) {
    	st.ct=strlen(s);
    	delete[] st.str;
    
    	st.str=new char[st.ct+1];
    	strcpy(st.str,s);
    }
    
    int main() {
    	Stringy test={NULL,0};
    	set(test,"squirrel");
    
    	std::cout<<test.str<<std::endl;
    }
    Last edited by Tux0r; 07-21-2009 at 06:39 AM.

  3. #3
    Novice
    Join Date
    Jul 2009
    Posts
    568
    Isn't the following undefined behavior, at least on the first run? Since I'm using delete on memory I did not allocate with new.
    Code:
    delete[] st.str;

  4. #4
    C++11 User Tux0r's Avatar
    Join Date
    Nov 2008
    Location
    Sweden
    Posts
    135
    Quote Originally Posted by msh View Post
    Isn't the following undefined behavior, at least on the first run? Since I'm using delete on memory I did not allocate with new.
    Code:
    delete[] st.str;
    In main() I initialized str to NULL

  5. #5
    and the hat of sweating
    Join Date
    Aug 2007
    Location
    Toronto, ON
    Posts
    3,545
    Why not make Stringy a class with constructors & destructors that can properly manage the member variables (and make the members private)?
    "I am probably the laziest programmer on the planet, a fact with which anyone who has ever seen my code will agree." - esbo, 11/15/2008

    "the internet is a scary place to be thats why i dont use it much." - billet, 03/17/2010

  6. #6
    C++11 User Tux0r's Avatar
    Join Date
    Nov 2008
    Location
    Sweden
    Posts
    135
    Quote Originally Posted by msh View Post
    Isn't the following undefined behavior, at least on the first run? Since I'm using delete on memory I did not allocate with new.
    Code:
    delete[] st.str;
    Also this is how you do it in C++, if you use classes

    Code:
    #include <iostream>
    #include <cstring>
    
    class Stringy {
        public:
            char* str;
            int len;
    
            Stringy(): str(),len() {}
    
            void set(const char*);
    };
    
    void Stringy::set(const char *s) {
    	len=strlen(s);
    	delete[] str;
    
    	str=new char[len+1];
    	strcpy(str,s);
    }
    
    int main() {
    	Stringy test; //default constructor called here
    
    	test.set("squirrel");
    	std::cout<<test.str<<std::endl;
    }
    edit: although note this is a poorly written class, it merely gives you an idea of basic init with classes
    Last edited by Tux0r; 07-21-2009 at 06:54 AM.

  7. #7
    Novice
    Join Date
    Jul 2009
    Posts
    568
    Quote Originally Posted by Tux0r View Post
    In main() I initialized str to NULL
    Ah. Forgot that it's OK to use delete on NULL pointers.

    (Classes are not available at this point. The book introduces them only at about half way through.)

  8. #8
    C++11 User Tux0r's Avatar
    Join Date
    Nov 2008
    Location
    Sweden
    Posts
    135
    Quote Originally Posted by msh View Post
    (Classes are not available at this point. The book introduces them only at about half way through.)
    Ok then you just got a little head start

  9. #9
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by Tux0r View Post
    Also this is how you do it in C++, if you use classes

    Code:
    #include <iostream>
    #include <cstring>
    
    class Stringy {
        public:
            char* str;
            int len;
    
            Stringy(): str(),len() {}
    
            void set(const char*);
    };
    
    void Stringy::set(const char *s) {
    	len=strlen(s);
    	delete[] str;
    
    	str=new char[len+1];
    	strcpy(str,s);
    }
    
    int main() {
    	Stringy test; //default constructor called here
    
    	test.set("squirrel");
    	std::cout<<test.str<<std::endl;
    }
    edit: although note this is a poorly written class, it merely gives you an idea of basic init with classes
    Forgot to delete in destructor.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  10. #10
    C++11 User Tux0r's Avatar
    Join Date
    Nov 2008
    Location
    Sweden
    Posts
    135
    heh

  11. #11
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    >> this is how you do it in C++
    In C++ I'd use an existing string class, and I'd use vector instead of new[]/delete[] anyway. But of course those are also beyond the scope of msh's current learning.

    >> Forgot to delete in destructor.
    And forgot the rule of three as well.

    BTW, is this C++ Primer by Lippman, et al or C++ Primer Plus by Prata?

  12. #12
    C++11 User Tux0r's Avatar
    Join Date
    Nov 2008
    Location
    Sweden
    Posts
    135
    lol I was not trying to write a Tux0r::String I was only showing msh how to not have to init structs manually...

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Memory leak in this case?
    By George2 in forum C++ Programming
    Replies: 3
    Last Post: 03-22-2008, 05:05 AM
  2. memory leak in the code?
    By George2 in forum C++ Programming
    Replies: 20
    Last Post: 01-13-2008, 06:50 AM
  3. Is this code memory leak free? ---> POSIX Threads
    By avalanche333 in forum C++ Programming
    Replies: 9
    Last Post: 04-13-2007, 03:19 PM
  4. Any Memory Leak Checking Tool?
    By George2 in forum C Programming
    Replies: 4
    Last Post: 06-21-2006, 11:02 PM
  5. Manipulating the Windows Clipboard
    By Johno in forum Windows Programming
    Replies: 2
    Last Post: 10-01-2002, 09:37 AM

Tags for this Thread