Segmentation fault when returning string from function in c++

Question

I have an error in my class. I'm suspect it's something simple, maybe me failing to understand how strings works in C++. My function tries to returns local std::string to another std::string, but instead of getting the string, I'm getting segmentation fault after the function returns. This is my code:

    TestClass::TestClass(std::string a, std::string b)
    {
        m_a = a;
        m_b = b;
    }

    std::string TestClass::stringHandler()
    {
        const char myTemplate[] = "a=%s,b=%s";
        char formattedString[strlen(myTemplate) + m_a.length() + m_b.length()];
        sprintf( formattedString, myTemplate, m_a.c_str(), m_b.c_str());
        std::cout << "formattedString= " << formattedString << "\n";
        std::string returnStr(formattedString);
        std::cout << "returnStr=" << returnStr << "\n";
        return returnStr;
    }

    int main(...)
    {
        /* getting a and b from argv */
        TestClass MyClassObj(a, b);
        std::string strRet = MyClassObj.stringHandler();
        std::cout << "Back after stringHandler\n";
        return 0
    }

In stringHandler, when I'm printing returnStr, it displayes properly. But right after that, I'm getting Segmentation fault, and "Back after stringHandler" is not being printed. Do any of you masters, have any idea what am I doing wrong?

Always use snprintf, never sprintf, to avoid buffer overruns. — John Zwinck
– John Zwinck, Commented Jul 31, 2014 at 5:20
Interesting choices for format strings in there... Why do I harbor doubts %b and %c are in your real code ? Assuming they're actually %s, what makes you think the a=, b=, etc., as well as the terminator, apparently take no space ? You're overwriting your string buffer and invoking undefined behavior. Finally, you have three format specifiers, yet only two actual arguments. Stop messing with sprintf and use a std::ostringstream. — WhozCraig
– WhozCraig, Commented Jul 31, 2014 at 5:21
Oops :), I fixed it to %s (as it is on my real code). a and b are a one word string (I verify that before going to stringHandler) — jfhfhf839
– jfhfhf839, Commented Jul 31, 2014 at 5:24
I'd suggest using an std::stringstream instead of sprintf and avoiding buffer overrun problems altogether if you're going to do a lot of operations. — Lander
– Lander, Commented Jul 31, 2014 at 5:29

M.M · Accepted Answer · 2014-07-31 05:36:08Z

3

Several issues:

%b is not a valid format specifier.
You have 3 format specifiers (if %b is fixed!) but you only passed 2 arguments.
You didn't allocate enough memory for formattedString.
Arrays with runtime size are illegal in C++

(update:) after the edits to OP, the first three of these were fixed, so if your compiler also has an extension for runtime array size then this code would not cause a segfault.

The simplest fix is just to write:

std::string returnStr = "a=" + m_a + ",b=" + m_b;

But supposing you want to stick with printf formatting, here is one way to do it. Although it is possible to calculate and work out the exact amount of space, that is fragile. It'd be easy to cause a buffer overflow if you made a change to myTemplate.

One plan would be to allocate a large amount of space. A more robust way is to use snprintf to determine the buffer size first:

char const myTemplate[] = "a=%s,b=%s";
size_t expect_len = std::snprintf(NULL, 0, myTemplate, m_a.c_str(), m_b.c_str());

if ( expect_len == -1 )
    throw std::runtime_error("snprintf failed");

std::vector<char> buffer(expect_len + 1);
std::snprintf(&buffer[0], buffer.size(), myTemplate, m_a.c_str(), m_b.c_str());

std::string returnStr(buffer);

In C++11 you could actually snprintf directly into returnStr.

To be clear, the whole printf idea is not very safe as you can cause undefined behaviour if myTemplate contains something you didn't expect. (I presume you did it this way to allow yourself to specify a different format string at runtime). So use it with caution.

edited Jul 31, 2014 at 5:36

answered Jul 31, 2014 at 5:29

M.M

143k26 gold badges223 silver badges400 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

jfhfhf839 Over a year ago

Thank you so much! Its working now. myTemplate is a predefined char const, but a very long one (it is a soap request actually, and I'm parsing the params into it before sending)

Collectives™ on Stack Overflow

Segmentation fault when returning string from function in c++

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related