CODEWITHSUNDEEP
c++stringvisual-studio-2015
Ofek .T.
Ofek .T.

Reputation: 762

Program crashes in the second run, after the return 0 from main C++

I have built my own functions of strlen and strdup. When i use my strdup in the first time it's okay, i close the window, run it again, then in the end of the program after the return 0 from the main the program crashes. VS just says that it triggered a breakpoint.

#include "stdafx.h"
#include <iostream>

using namespace std;

int MyStrlen(const char* str);
char* MyStrdup(const char* str);

int main()
{
    char *s1 = "Hello World!";
    char *s2 = MyStrdup(s1);

    cout << s1 << " , " << s2 << endl;
    system("pause");
    return 0;
}


int MyStrlen(const char* str)
{
    register int iLength = 0;

    while (str[iLength] != NULL)
    {
        iLength++;
    }

    return iLength;
}

char* MyStrdup(const char* str)
{
    char* newStr;
    int strLength = MyStrlen(str);

    newStr = new char(strLength+1);

    for (register int i = 0; i < strLength; i++)
    {
        newStr[i] = str[i];
    }
    newStr[strLength] = NULL;

    return newStr;
}

Can someone note the place that makes it crash? I think it's a memory leak maybe. Also, can you note things to improve in the code? For my learning purpose

EDIT: Thanks, I don't know why I used () instead of [] to define my new char[]. That was a memory leak or overwrite after all.

Upvotes: 1

Views: 948

Answers (2)

Adi Levin
Adi Levin

Reputation: 5233

The "new" statement for an array should be with square brackets:

newStr = new char[strLength+1];

When you do

new char(c)

It allocates a single character and copies the character c into it. When you do

new char[n]

it allocates memory for n characters

Upvotes: 11

Some programmer dude
Some programmer dude

Reputation: 409166

The expression new char(strLength+1) allocates a single character, and initializes it to strLength + 1. That of course means you will write out of bounds and have undefined behavior when you copy the string.

You should use new char[strLength + 1] instead, to allocate an "array" of characters.

On an unrelated note, while the terminating character in a string is commonly called the null character, it's not actually a null pointer (which is what NULL is for). Not that it really matters since in C++ NULL is a macro that expands to 0, but you should probably be explicit and use '\0' anyway (it gives more context for future readers).

Upvotes: 4

Related Questions