Alex Díaz
Alex Díaz

Reputation: 2363

Printing out std::string crashes with segmentation fault error

I was trying to improve my reply to another question here How to cut the content of a string till a particular string or position? to use pointer arithmetic and std::substr so I ended up with the following

#include <iostream>
#include <string>
#include <cstring>

std::string getString(std::string fullString){
    char string[80];
    char* endString;
    strcpy(string, fullString.c_str());
    strtok(string, "|");
    for(int i = 0; i < 4; i++){
        endString = strtok(NULL, "|");
    }
    return fullString.substr(endString - string, std::string::npos);
}
int main( void ){
    std::string str("{[(2015/11/30|01:07:53.357|-1227639088|DefaultThread|./src/Myprogram.cpp:564|int main(int, argv**))]} Server Starting....");
    std::cout << getString(str) << std::endl;
    return 0;
}

However that crashes with a segmentation fault error, if I change it to

#include <iostream>
#include <string>
#include <cstring>

std::string getString(std::string fullString){
    char string[80];
    char* endString;
    strcpy(string, fullString.c_str());
    strtok(string, "|");
    for(int i = 0; i < 4; i++){
        endString = strtok(NULL, "|");
    }
    std::cout << fullString.substr(endString - string, std::string::npos) << std::endl;
    return fullString.substr(endString - string, std::string::npos);
}
int main( void ){
    std::string str("{[(2015/11/30|01:07:53.357|-1227639088|DefaultThread|./src/Myprogram.cpp:564|int main(int, argv**))]} Server Starting....");
    std::cout << getString(str) << std::endl;
    return 0;
}

The program runs fine and the output is as expected

./src/Myprogram.cpp:564|int main(int, argv**))]} Server Starting....
./src/Myprogram.cpp:564|int main(int, argv**))]} Server Starting....

why does it crash on the first scenario?

Upvotes: 2

Views: 1844

Answers (2)

Vlad from Moscow
Vlad from Moscow

Reputation: 311088

Your program has undefined behaviour because the length of the string stored in str is greater than 80 characters. Thus in this statement

strcpy(string, fullString.c_str());

you overwrite memory beyond array string.

Moverover it is a bad approach to use C function strtok instead of member function find (or rfind) of class std::string.

Also it is not clear why exactly magic number 4 is used in the loop

for(int i = 0; i < 4; i++){
    endString = strtok(NULL, "|");
}

Upvotes: 2

Lightness Races in Orbit
Lightness Races in Orbit

Reputation: 385395

Your input string is 120 bytes wide. Your C-string buffer is 80 bytes wide. Hrmm.

Use the find functions instead of this error-prone C nonsense!!

To get everything since the last |:

#include <iostream>
#include <string>
#include <stdexcept>

std::string getString(const std::string& fullString)
{
    size_t pos = fullString.rfind('|');
    if (pos == std::string::npos)
       throw std::runtime_error("Could not find a '|'!");

    return fullString.substr(pos+1);
}

int main()
{
    std::string str("{[(2015/11/30|01:07:53.357|-1227639088|DefaultThread|./src/Myprogram.cpp:564|int main(int, argv**))]} Server Starting....");
    std::cout << getString(str) << std::endl;
}

(live demo)

Adjust as needed to scan back for the 2nd, 3rd, 4th |.

Upvotes: 4

Related Questions