0

Hi In my code i have a string stored in char* i want to copy only some part of the string to another char*. say for example,

    char * str1 = "I am new to c++";

    // I am creating new char pointer
    char * str2 = new char[20];

    int startpos = 5;

    int destpos = 0;

    // i am checking for the white space from the start posistion
    while(str1[startpos]!=' '){

    str2 [destpos++] = str1[startpos++];

    }

    printf("\n New String is %s",str2);

    // now i am deleting the char*
    delete[] str2;

    delete[] str1;

I am using this kind of manipulation and this code runs properly sometimes and causes seg fault (double free or corruption).

Someone please tell me what is the reason.

Karthik Kalyanasundaram
  • 1,525
  • 11
  • 24
user2128472
  • 1
  • 1
  • 1
    Why don’t you use `std::string`? Manual resource management is highly error-prone. –  Feb 16 '14 at 10:30
  • 1
    You never terminated your target string, and make absolutely zero affordances on potential overrun. Deleting a read-only string literal is just the icing on the cake. – WhozCraig Feb 16 '14 at 10:31

5 Answers5

3

First of all, never assign a string literal to a pointer to non-const char:

char* str1 = "I am new to c++"; // bad
char const* str1 = "I am new to c++"; // good

Secondly, never delete[] string literals.

If you want sanity, use std::string:

std::string str1("I am new to c++");

int startpos = 5;

std::string str2;
for (auto it = str1.begin() + startpos; it != str1.end(); ++it) {
    if (*it == ' ')
        break;
    str2.push_back(*it);
}

std::cout << "\n New String is " << str2;

See std::string for more information.

Whenever you use new and delete, you’re doing it wrong™.

1
delete[] str1;

is undefined behavior, because str1 is a string literal, not a char pointer initialized with a new-allocated char array (new char[N]). This is probably why your program ends with a segmentation fault.

As other answers have suggested, you should probably be using a std::string instead anyway.

Martin J.
  • 5,028
  • 4
  • 24
  • 41
0

Use strncpy as below to copy strings,

memset(str2, 0, strlen(str2)); 
strncpy(str2 + destpos, str1 + startpos, sizeof(strlen(str1) - startpos - 1));
Sunil Bojanapally
  • 12,528
  • 4
  • 33
  • 46
  • This uses `sizeof()` incorrectly, and makes no guarantees `str2` is zero-terminated. As written it will copy at-most a number of chars equal to the byte-size of a `size_t` type. And if the source string is equal-to or longer than that, will not terminate the target string. – WhozCraig Feb 16 '14 at 10:34
  • @WhozCraig Agreed. Can we fix it with `memset` destination str to 0 before performing `strncpy` – Sunil Bojanapally Feb 16 '14 at 10:48
  • No, you fix it by simply terminating the string. `strncpy` is only a string handling function in that it will tail-fill with 0 if the source is smaller than the max-size specified. If it is larger, no 0-padding is done, including no terminator. using `memset` is utter overkill and ultimately still won't solve the problem as you have used it, since you're now using the length of an uninitialized indeterminate buffer as the length to set. [Read this for more info](http://stackoverflow.com/questions/14065391/strncpy-leading-to-segmentation-fault/14067026#14067026). – WhozCraig Feb 16 '14 at 12:40
0

Since this is C++, for the love of God, please use std::string.

(P.S., it is failing because "str1" was assigned the address of read-only memory; it wasn't dynamically allocated with new[] or malloc. Thus, it should not be passed to delete[] or free()).

Michael Aaron Safyan
  • 93,612
  • 16
  • 138
  • 200
0

delete[] str1; - this is the line which gives you segmentation fault. str1 is declared as constant string literal. No need to call delete for str1

Karthik Kalyanasundaram
  • 1,525
  • 11
  • 24