CODEWITHSUNDEEP
c++
jfhfhf839
jfhfhf839

Reputation: 1739

Segmentation fault when returning string from function in c++

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?

Upvotes: 2

Views: 2107

Answers (1)

M.M
M.M

Reputation: 141554

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.

Upvotes: 3

Related Questions