Heap corruption while freeing memory
Solution 1
Change the code to:
argv_[i] = (char*) malloc(strlen(argv[i]) + 1) ;
strcpy(argv_[i], argv[i]);
It's because your StrCpy likely trashes your memory. You have to account for the terminating nul byte as well, provided your StrCpy works like the standard strcpy (which it has to in order to be useful, better just use the standard strcpy() unless you have a good reason not to).
sizeof(char) is by definition 1, so that can be omitted too.
Solution 2
You need to allocate one character more than the strlen of a C-string if you want to copy it. This is because strlen does not count the termination null-character.
Solution 3
Please use strdup() - it allocates the right amount of memory and copies characters for you.
Eternal Learner
Updated on June 14, 2022Comments
-
Eternal Learner about 2 years
I have a class as follows
struct CliHandler { CliHandler(int argc, char** argv); ~CliHandler(); int doWork(); int argc_; char** argv_; private: CliHandler(const CliHandler&){} CliHandler& operator=(const CliHandler&){} };
//Constructor
CliHandler::CliHandler(int argc, char** argv) { //set command line parameters argc_ = argc; argv_ = (char**) malloc(argc_ * sizeof(char*)); for(int i=0; i<argc_; ++i) { std::cout<<sizeof(argv[i]); argv_[i] = (char*) malloc(strlen(argv[i]) * sizeof(char)); StrCpy(argv_[i], argv[i]); } }
// destructor
CliHandler::~CliHandler() { for(int i=0; i<argc_; ++i) free(argv_[i]); free(argv_); }
While I debug, I get an error " Heap corruption detected. CRT detected that application wrote to memory after end of heap buffer. " My question id "Where exactly am i making a mistake ? How do I fix it". I am using visual stdio 2008.
Edit:I did something like this to add 1
argv_[i] = (char*) malloc(strlen(argv[i] + 1) * sizeof(char));
Which is terrible as it increments the pointer argv[i] by one. My co-worker pointed out that subtle issue. It should be
argv_[i] = (char*) malloc( (strlen(argv[i]) + 1) * sizeof(char));
-
Marius Bancila about 13 yearsOne thing that I see is that you don't allocate space for the null-terminator. Should be
strlen(argv[i]) + 1
. -
Admin about 13 yearsWhy, if you are using C++, are you using malloc? And why are you not using std:;vector and std::string?
-
Drew Delano about 13 yearsAnother potential problem I see is that you don't follow the Rule of Three. If any copying happens, you're in trouble.
-
Eternal Learner about 13 years@unapersson - i'm integrating some function with legacy code.
-
Admin about 13 yearsSo what? The things you allocate to are private, so they can't be used directly by the legacy code, so you could (and should) implement them using vectors and strings. Otherwise, you are simply writing MORE legacy code.
-
-
karlphillip about 13 yearsSummarizing, he needs to add space for the extra byte on the 2nd
malloc()
-
Admin about 13 years@Arkadiy Non-standard, impossible to tell how to de-allocate, std::string is an infinitely better way of doing things, .....
-
Admin about 13 yearsit's part of Posix standard, and you deallocate with free(). Agreed that std::string is the right way to do it.
-
vpalmu about 13 yearsThe guy is explicitly using C style strings here. If that's what he wants, strdup() is fine. It's in the C standard library.
-
Jason about 13 years@Joshua:
strdup()
is not part the C standard library. It is part of POSIX.1-2001. -
Jason about 13 years@unapersson:
strdup
isn't non-standard. It's just not part of the C standard. Instead it is part of the POSIX standard which specifically says you need to tofree
to deallocate it. -
Admin about 13 years@Evan If this question was tagged POSIX, that might be a good point. Unfortunately, it isn't.
-
Jason about 13 years@unapersson: My point is that the claim "impossible to tell how to de-allocate" isn't true. The POSIX standard is very specific about it. I agree it isn't standard C, and therefore is less portable. But the reality is that POSIX is a fairly widely accepted standard. If you happen to be targeting a POSIX platform, there is nothing wrong with using it.