Run-time error when allocating new char[]

[leonid1990]

Hi people,
the code below describes an example of implementation for couple functions of built-in C++ string class, as the CTOR and insert().
when I run this code, a run-time error occurs when b.set(r) is executed. More precisely, when the program tries to allocate str = new char[l] (only for b, not for a as said before). It says that "ConsoleApplication.exe has triggered a breakpoint."
Why? Particularly, why a is OK and b isn't?

#include <iostream>
using namespace std;

class String
{
private:
    char* str;

public:

    String(char* s = NULL);
    ~String();
    void set(char *s);
   
    String& insert(int index, const char*other, String* y);
};

String::String(char *s)
{
    if (s)
    {
	int l = strlen(s) + 1;
	str = new char[l];
	strcpy_s(str, 100, s);
    }
    else
	str = NULL;
}

String :: ~String()
{
    if (str)
    {
	delete[] str;
	str = NULL;
    }

}

void String::set(char*s)
{
    if (s)
    {
	int l = strlen(s) + 1;
	str = new char[l];
	strcpy_s(str, 100, s);
    }
}

String& String::insert(int index, const char* other, String* y)
{
    int n = strlen(str) + strlen(other) + 1;
    char* tmp = new char[n];//New dynamic space allocation.
    strcpy_s(tmp, 100, other);//Copy the string are sent to help.
    strcpy_s(tmp + index, 100, str);/*Copying string ptr end String helped without overwriting.*/
    strcpy_s(tmp + strlen(tmp), 100, other + index);/*Copying the end of the word string sent without overwriting data.*/
    *y = tmp;

    delete[]tmp;
    return *y;
}


int main()
{
    String a, b;
    char * t = new char[1000];
    char * r = new char[1000];
    cin >> t;
    cin >> r;

    a.set(t);
    b.set(r);

    return 0;
}

[ne555]

> strcpy_s(str, 100, s);
¿why 100?
¿what does that parameter mean?

[leonid1990]

100 is to explicitly specify the size of the target buffer, so the function will not overflow. I chose it arbitrary.
Visual Studio 2013 deprecated strcpy totally, and treats it as compilation error, so I used strcpy_s instead as it says.

[ne555]

> 100 is to explicitly specify the size of the target buffer
ah, ¿so your target has that size?


here's a hint: it doesn't.
str = new char[l];

[leonid1990]

First, as I understand - 100 is the maximum size of the buffer.
But anyway, why a.set(t) works while b.set(r) doesn't?

[ne555]

Suppose that you input

hello
world

`l' would be 6, but you are telling to write 100 elements to `str'. I don't understand why you are surprised then.

[leonid1990]

Again, if I understand correctly - I'm telling to write at most 100 elements to str.
I'm surprised of the fact that a.set() works well but b.set() doesn't...

[Chervil]

I'm telling to write at most 100 elements to str.

According to the documentation page,
https://msdn.microsoft.com/en-us/library/td1esda9.aspx

The debug versions of these functions first fill the buffer with 0xFE.

That is, the function will always write 100 bytes to the buffer, before copying the characters from the source string. The whole point of specifying a number is to tell the function the size of the buffer, rather than give it some other unrelated number. That will result in a buffer overrun, exactly the issue that the function was supposed to prevent ... resulting in undefined behaviour.

[leonid1990]

OK, but anyway I'm curious to know why a.set(t) works and b.set(r) doesn't?

[Chervil]

Undefined behaviour is undefined - anything can happen.

[leonid1990]

Alright, thank you guys