Like Tree1Likes
  • 1 Post By Salem

random crashes on delete []

This is a discussion on random crashes on delete [] within the C++ Programming forums, part of the General Programming Boards category; Hi, im having trouble trying to write the String class, following along with "C++ without Fear". and most of the ...

  1. #1
    Registered User
    Join Date
    Jun 2012
    Posts
    2

    Unhappy random crashes on delete []

    Hi, im having trouble trying to write the String class, following along with "C++ without Fear". and most of the time my destructor for my string class crashes on delete [] ptr;

    heres the sources.

    main.cpp
    Code:
    #include "string.h"
    //#include <iostream>
    //#include <windows.h>
    
    using namespace std;
    
    int main(int argc, char** argv) {
        String str("hello world"); //crashes on this Strings destructor.
        String str2;
        String str3;
        String tmp;
        char *c = new char[500]; 
        delete c; //works fine.
        //cin.get();
        return 0;
    }
    string.h
    Code:
    #ifndef STRING_H
    #define    STRING_H
    
    //#include <cstring>
    //#include <iostream>
    
    class String {
    private:
        char *ptr;
    public:
        String();
        String(const char *s);
        String(const String &src);
        ~String();
        operator char*(){return ptr;}
        int operator==(const String &other);
        int operator>(const String &other);
        int operator<(const String &other);
        String& operator=(char *s);
        String& operator=(String &src);
    private:
        void cpy(char *s);
    };
    
    
    #endif    /* STRING_H */
    string.cpp
    Code:
    #include "string.h"
    
    int strlen(const char *str){
        int i = 0;
        while(str[i] != '\0')
            i++;
        return i;
    }
    
    int strcmp(const char *c1, const char *c2){
        int i = 0;
        while(c1[i] == c2[i]){
            if(c1[i] == '\0')
                return 0;
            i++;
        }
        if(c1[i] > c2[i])
            return 1;
        if(c1[i] < c2[i])
            return -1;
    }
    
    void strcpy(char *d, const char *s){
        int i = 0;
        while(s[i] != '\0'){
            d[i] = s[i];
            i++;
        }
        d[i] = '\0';
    }
    
    String::~String(){
        delete []ptr;  //CRASHES HERE <-------------
    }
    
    String::String(){
        ptr = new char[1];
        ptr[0] = '\0';
    }
    
    String::String(const char *s){
        int n = strlen(s);
        ptr = new char(n+1);
        strcpy(ptr, s);
    }
    
    String::String(const String& src){
        int n = strlen(src.ptr);
        ptr = new char(n+1);
        strcpy(ptr, src.ptr);
    }
    
    int String::operator ==(const String& other){
        return(strcmp(ptr, other.ptr) == 0);
    }
    
    int String::operator <(const String& other){
        if(strcmp(ptr, other.ptr) == -1)
            return 1;
        else
            return 0;
    }
    
    int String::operator >(const String& other){
        if(strcmp(ptr, other.ptr) == 1)
            return 1;
        else
            return 0;
    }
    
    void String::cpy(char* s){
        delete []ptr;
        int n = strlen(s);
        ptr = new char[n + 1];
        strcpy(ptr, s);
    }
    
    String& String::operator =(char* s){
        cpy(s);
        return *this;
    }
    
    String& String::operator=(String &src){
        cpy(src.ptr);
        return *this;
    }
    heres the call stack when it crashes

    Code:
    ntdll!TpWaitForAlpcCompletion ()
    ?? ()
    ntdll!RtlLargeIntegerDivide ()
    ?? ()
    ntdll!RtlCopyExtendedContext ()
    ?? ()
    i know it shouldnt be my strcpy, strcmp, strlen functions because i tried it using the standard functions and no change.

    another thing to note is that it only crashes when the String::String(const char *s); constructor is used.
    thanks for any help.

  2. #2
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,558
    > ptr = new char(n+1);
    Creates a single char, and initialises it with n+1

    ptr = new char[n+1];
    Creates an array of n+1 chars
    FumiTomii likes this.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  3. #3
    Registered User
    Join Date
    Jun 2012
    Posts
    2
    wow how did i miss that. id spent hours trying to figure out was was wrong, lol. it works now. thanks for the help.

  4. #4
    Registered User antred's Avatar
    Join Date
    Apr 2012
    Location
    Germany
    Posts
    252
    Quote Originally Posted by Salem View Post
    > ptr = new char(n+1);
    Creates a single char, and initialises it with n+1

    ptr = new char[n+1];
    Creates an array of n+1 chars
    Also, what's the point of dynamically allocating a character array of size 1 just so that you can put a terminating null char in it to indicate that the string is empty? Why not just set ptr to null and be done with it? You would have to adjust the logic of some of your methods, but I think it would be worth it.

  5. #5
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,633
    Your strlen returns length + 1, which is wrong.
    Your strcmp returns wrong if str1 is shorter than str2.
    Your strcpy is a horrible, horrible function. You have your class, so why do you have strcpy? Remove it!
    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.

  6. #6
    Registered User antred's Avatar
    Join Date
    Apr 2012
    Location
    Germany
    Posts
    252
    Quote Originally Posted by Elysia View Post
    Your strcpy is a horrible, horrible function. You have your class, so why do you have strcpy? Remove it!
    I think he's only using it as an implementation detail to copy string data between instances. Notice that there is no mention of strcpy in his header file.

  7. #7
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,633
    In such case, I'd recommend breaking out the whole copy procedure into its own function and make it a private member.
    strcpy by itself is very, very dangerous. If insisting on making a private strcpy, at least provide buffer sizes to make sure you don't cause buffer overflows.
    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.

  8. #8
    Registered User antred's Avatar
    Join Date
    Apr 2012
    Location
    Germany
    Posts
    252
    There are various const-correctness issues with his code as well. Plus, operator ==, < and > should return a bool, not an int. I'll post a modified version of the original header file to show what the signature of the various methods should be:

    Code:
    #ifndef STRING_H
    #define    STRING_H
    
    //#include <cstring>
    //#include <iostream>
    
    class String
    {
    private:
        char *ptr;
        
    public:
        String();
        String(const char *s);
        String(const String &src);
        ~String();
        operator char*(){return ptr;} // <-- If I were you, I'd get rid of this one.
                                      // Giving the outside world non-const access
                                      // to the string's internal data is a
                                      // a horrible idea.
    
        operator const char*() const {return ptr;}
        bool operator==(const String &other) const;
        bool operator>(const String &other) const;
        bool operator<(const String &other) const;
        String& operator=(const char *s);
        String& operator=(const String &src);
    private:
        void cpy(const char *s);
    };
    
    #endif    /* STRING_H */
    Last edited by antred; 06-21-2012 at 11:48 AM.

  9. #9
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,780
    antred makes a good point concerning your String class' conversion function to char*, but also, consider either declaring the conversion function to const char* to be explicit (a feature from C++11), or use a named conversion function instead (like std::string's c_str member function). Since you already allow implicit conversions from const char* to String, allowing implicit conversions from String to const char* can make things confusing.

    Another thing: your class definition does not need anything from <cstring> or <iostream>, so you should move those header inclusions into the source file instead (though I notice that you commented them out). Actually, you should not need <iostream> at all. You might want to overloaded operator>> and operator<< for input/output streams, in which case you should #include <iosfwd> in the header then #include <istream> and #include <ostream> in the source file.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Random crashes? huh?
    By cpjust in forum C++ Programming
    Replies: 28
    Last Post: 07-09-2008, 08:41 PM
  2. Concerning delete/delete[] at program exit
    By laserlight in forum C++ Programming
    Replies: 58
    Last Post: 01-09-2008, 12:40 PM
  3. Calling delete Crashes my Program
    By thetinman in forum C++ Programming
    Replies: 19
    Last Post: 10-13-2007, 03:07 AM
  4. Replies: 17
    Last Post: 11-16-2006, 08:06 PM
  5. Replies: 1
    Last Post: 12-14-2002, 12:51 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21