Delete[] Error...

This is a discussion on Delete[] Error... within the C++ Programming forums, part of the General Programming Boards category; I just threw together a quick exception handling class, and when I use it, I'm getting an error for some ...

  1. #1
    C++ Developer XSquared's Avatar
    Join Date
    Jun 2002
    Location
    Ontario, Canada
    Posts
    2,718

    Delete[] Error...

    I just threw together a quick exception handling class, and when I use it, I'm getting an error for some odd reason.

    Here's the code:

    vectorexception.h
    Code:
    #ifndef VECTOREXCEPTION_H_
    #define VECTOREXCEPTION_H_
    
    class VectorException {
    
      protected:
    
        char *error;
    
      public:
    
        VectorException();
        VectorException(const char *);
        ~VectorException();
    
        char *getError();
    
    };
    
    #endif

    vectorexception.cc
    Code:
    #include <cstring>
    #include "vectorexception.h"
    
    VectorException::VectorException() {
    
      error =  new char[sizeof(char) * strlen("No error message specified.")];
      strcpy(error,"No error message specified.");
    
    }
    
    VectorException::VectorException(const char *error) {
    
      this->error = new char[sizeof(char) * strlen(error)];
      strcpy(this->error,error);
    
    }
    
    VectorException::~VectorException() {
    
      delete[] error;
      error = NULL;
    
    }
    
    char *VectorException::getError() {
    
      return error;
    
    }
    main.cc
    Code:
    #include <cstdio>
    #include "vectorexception.h"
    
    int main(void) {
    
      try{
        throw VectorException("oh noes");
      }
      catch(VectorException e) {
        printf("%s\n",e.getError());
      }
    
      return 0;
    
    }
    And the error:
    Code:
    oh noes
    *** glibc detected *** double free or corruption (fasttop): 0x0805f158 ***
    Aborted
    I'm using GCC, and compiling with the flags -MMD -ansi -Wall -pedantic.
    Naturally I didn't feel inspired enough to read all the links for you, since I already slaved away for long hours under a blistering sun pressing the search button after typing four whole words! - Quzah

    You. Fetch me my copy of the Wall Street Journal. You two, fight to the death - Stewie

  2. #2
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    You forgot to count the \0 at the end
    strlen("string")+1 is what you want.
    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
    C++ Developer XSquared's Avatar
    Join Date
    Jun 2002
    Location
    Ontario, Canada
    Posts
    2,718
    Thanks for pointing that out, but it's still throwing the glibc error, even after changing those two lines.
    Naturally I didn't feel inspired enough to read all the links for you, since I already slaved away for long hours under a blistering sun pressing the search button after typing four whole words! - Quzah

    You. Fetch me my copy of the Wall Street Journal. You two, fight to the death - Stewie

  4. #4
    C++ Developer XSquared's Avatar
    Join Date
    Jun 2002
    Location
    Ontario, Canada
    Posts
    2,718
    This is really odd... I just stepped through my code using GDB, and I got the exception at the very end of the constructor, after all the code was done executing.
    Naturally I didn't feel inspired enough to read all the links for you, since I already slaved away for long hours under a blistering sun pressing the search button after typing four whole words! - Quzah

    You. Fetch me my copy of the Wall Street Journal. You two, fight to the death - Stewie

  5. #5
    Magically delicious LuckY's Avatar
    Join Date
    Oct 2001
    Posts
    856
    You need a copy constructor (to perform the necessary "deep copy") or use an STL string instead of char* so there are no other implications when "error" is copied from one object to another.
    Last edited by LuckY; 08-05-2005 at 03:53 PM.

  6. #6
    carry on JaWiB's Avatar
    Join Date
    Feb 2003
    Location
    Seattle, WA
    Posts
    1,972
    Is there a reason to even bother with dynamically allocating the string? Why not do something like this:
    Code:
    class Error
    {
    public:
      Error(const char* error):err(error) { }
      ~Error() {  }
      const char* what() { return err; }
    private:
      const char* err;
    };
    
    int main()
    {
    try
    {
      throw Error("Error!");
    }
    catch(const Error& someErr)
    {
     std::cout<<someErr.what();
    }
    
    }
    "Think not but that I know these things; or think
    I know them not: not therefore am I short
    Of knowing what I ought."
    -John Milton, Paradise Regained (1671)

    "Work hard and it might happen."
    -XSquared

  7. #7
    C++ Developer XSquared's Avatar
    Join Date
    Jun 2002
    Location
    Ontario, Canada
    Posts
    2,718
    Thank you muchly, Lucky. That fixed it.

    Edit: At JaWiB: I used pointers because I didn't think about using that method. Plus, it allows for non-hardcoded error messages (assuming I get rid of the const).
    Last edited by XSquared; 08-05-2005 at 03:08 PM.
    Naturally I didn't feel inspired enough to read all the links for you, since I already slaved away for long hours under a blistering sun pressing the search button after typing four whole words! - Quzah

    You. Fetch me my copy of the Wall Street Journal. You two, fight to the death - Stewie

  8. #8
    Magically delicious LuckY's Avatar
    Join Date
    Oct 2001
    Posts
    856
    Code:
    #ifndef VECTOREXCEPTION_H_
    #define VECTOREXCEPTION_H_
    
    class VectorException {
    
      protected:
        char *error;
    
      public:
    
        VectorException(const VectorException &rhs);    
        VectorException();
        VectorException(const char *);
        ~VectorException();
    
        const char *getError();
    
    };
    
    #endif
    
    ////////////////////////////////////////////////////////////////////////////////
    
    #include <cstring>
    #include "vectorexception.h"
    
    VectorException::VectorException(const VectorException &rhs) {
      error = new char[sizeof(char) * strlen(rhs.error) + 1];
      strcpy(error, rhs.error);
    }
        
    VectorException::VectorException() {
      error =  new char[sizeof(char) * strlen("No error message specified.") + 1];
      strcpy(error,"No error message specified.");
    }
    
    VectorException::VectorException(const char *error) {
      this->error = new char[sizeof(char) * strlen(error) + 1];
      strcpy(this->error,error);
    }
    
    VectorException::~VectorException() {
      delete[] error;
      error = NULL;
    }
    
    const char* VectorException::getError() {
      return error;
    }
    
    ////////////////////////////////////////////////////////////////////////////////
    
    #include <cstdio>
    #include "vectorexception.h"
    
    int main(void) {
    
      try {
        throw VectorException("oh noes");
      }
      catch(VectorException e) {
        printf("%s\n",e.getError());
      }
    
      return 0;
    
    }

  9. #9
    Registered User
    Join Date
    Jan 2005
    Posts
    7,317
    You might as well add a copy assignment operator to that while you're at it. Otherwise it will cause the same problem as the lack of a copy constructor did. The assignment operator would have to delete the currently stored error string, or use the swap trick to let the destructor do it automatically.

    You should catch exceptions by reference, there's no reason not to and in this case might save some unnecessary copying.

    Using the C++ string class would mean that the copy constructor, copy assignment operator and destructor wouldn't even be necessary for your class. It would just work.

  10. #10
    Registered User
    Join Date
    Nov 2002
    Posts
    491
    Use std::string and the sizeof is not required for new, new already knows what size each object is, it just needs to know how many to make.

  11. #11
    Magically delicious LuckY's Avatar
    Join Date
    Oct 2001
    Posts
    856
    Quote Originally Posted by orbitz
    Use std::string and the sizeof is not required for new, new already knows what size each object is, it just needs to know how many to make.
    That's only true under the assumption that it will remain a char. I, myself, was assuming that in his actual class he might be opting to use TCHARs for full compatibility. And, of course, if he is, he'd have to use _tcscpy() and _tcslen() instead of strcpy() and strlen(), respectively.

  12. #12
    Registered User
    Join Date
    Jun 2005
    Posts
    6,208
    As a general rule, it is not a good idea for any type that you throw as an exception to use dynamic memory allocation. The reason is that the process of creating the exception, or copying it around (which is potentially required during stack unwinding) can throw an exception. That can result in calling code receiving an exception it doesn't expect (eg if the memory allocation fails) or [depending on how it happens] either undefined behaviour or a call to abort() if a second exception gets thrown while the first is active.

    That also means that using std::string within an exception type is a bad idea; std::string uses dynamic memory.

    The example given by JaWib (which I repeat here) ...
    Code:
    class Error
    {
    public:
      Error(const char* error):err(error) { }
      ~Error() {  }
      const char* what() { return err; }
    private:
      const char* err;
    };
    
    int main()
    {
    try
    {
      throw Error("Error!");
    }
    catch(const Error& someErr)
    {
     std::cout<<someErr.what();
    }
    
    }
    is also a problem as it implicitly passes a copy of a local string around in the exception. The result may not show up in the specific example (the exception is both thrown and caught in main()), but yields undefined behaviour. To make it obvious, consider that happens if the exception is thrown from a function called by main(): e.GetError() will point to a string that was in scope at the throw point, but not when it is caught.

    The more usual way to do it is;
    Code:
    class Error
    {
    public:
      Error(const char* error) {if (error != NULL) strncpy(err, error, 29); else strcpy(err, "Uknown error");}
      ~Error() {  }
      const char* what() const { return err; }
    private:
      char err[30];
    };
    
    int main()
    {
    try
    {
      throw Error("Error!");
    }
    catch(const Error& someErr)
    {
     std::cout<<someErr.what();
    }
    
    }

  13. #13
    Registered User
    Join Date
    Jan 2005
    Posts
    7,317
    If "Error!" is a constant string literal, then it won't go out of scope, so JaWiB's example is fine, correct?

  14. #14
    Registered User
    Join Date
    Jun 2005
    Posts
    6,208
    Quote Originally Posted by Daved
    If "Error!" is a constant string literal, then it won't go out of scope, so JaWiB's example is fine, correct?
    No.

    Strictly speaking (the letter of the standard) it is out of scope for the caller and therefore an access to it (particularly dereferencing it) yields undefined behaviour. In practice, what you say is often true, but that's because compilers and linkers do not tend to truncate lifetimes of constants if there is enough free resources around to allow them to keep existing without thrashing the system. But on systems which are short of memory, the compiler and linker can (and will) purge from memory or caches anything that is no longer required to exist --- and local constant strings are a prime candidate for that.

  15. #15
    Toaster Zach L.'s Avatar
    Join Date
    Aug 2001
    Posts
    2,686
    Hmm... How does std::exception work (in general), then? I had the same idea in my head that Daved did.
    The word rap as it applies to music is the result of a peculiar phonological rule which has stripped the word of its initial voiceless velar stop.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Getting an error with OpenGL: collect2: ld returned 1 exit status
    By Lorgon Jortle in forum C++ Programming
    Replies: 6
    Last Post: 05-08-2009, 08:18 PM
  2. An error is driving me nuts!
    By ulillillia in forum C Programming
    Replies: 5
    Last Post: 04-04-2009, 09:15 PM
  3. Making C DLL using MSVC++ 2005
    By chico1st in forum C Programming
    Replies: 26
    Last Post: 05-28-2008, 01:17 PM
  4. Connecting to a mysql server and querying problem
    By Diod in forum C++ Programming
    Replies: 8
    Last Post: 02-13-2006, 09:33 AM
  5. Couple C questions :)
    By Divx in forum C Programming
    Replies: 5
    Last Post: 01-28-2003, 12:10 AM

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