0

I'm relatively new to C++ and the Windows API (coming from a Java background) and I was just playing around with the Windows API calling MessageBox and other simple functions until I tried to pass a concatenated string from a custom function to MessageBox where I noticed a weird output in the generated window.

This is the suspicious function:

const char* addFoo(const char* strInput)
{
    return ("foo-" + std::string(strInput)).c_str();
}

It just returns the original input with a foo- added in front. (I hope I'm not doing anything incredibly wrong there)

In main I then do two calls to MessageBox first without calling the function but instead doing all the calculation on the fly, and afterwards calling the function:

const char* a = "bar";
MessageBox(NULL, ("foo-" + std::string(a)).c_str(), "The Foobar Experiment", MB_OK);
MessageBox(NULL, addFoo(a), "The Foobar Experiment", MB_OK);

This is the result I get by doing the string concatenation on the fly (case 1):

Calling MessageBox without calling my addFoo function

The result I get by calling the function addFoo (case 2):

Calling MessageBox by passing the addFoo result directly

Does anyone have any idea why I'm getting these unreadable characters on the generated window by using my addFoo function? Thanks in advance and sorry for the long post.

Community
  • 1
  • 1
Alex Koukoulas
  • 998
  • 1
  • 7
  • 21
  • UB. Your std::string is no longer in existence (local to the addFoo()) when MessageBox uses the pointer to the char array. – drescherjm Apr 07 '15 at 18:31
  • 2
    `coming from a Java background` Which means very simply to not use Java as a guide in writing C++ code. – PaulMcKenzie Apr 07 '15 at 18:35

2 Answers2

2

There are two fundamentally wrong things in your code, one being C++ related, the other being Windows related.

First, you are returning a pointer to a local entity, namely the return value of c_str() which is a pointer. Returning pointers to local variables is undefined behavior. What you want to do is return a string, not a pointer. In C++, there are string types such as std::string and std::wstring that implement the correct copy semantics that are required to have objects returned safely without error.

#include <string>
std::string addFoo(const char* strInput)
{
    return "foo-" + std::string(strInput);
}

The second thing wrong with your code is that in the Windows world, you have basically two types of applications with respect to character type. You have the MBCS application, and Unicode application.

If you built a Unicode application, your calls to MessageBox would not have compiled successfully, since MessageBox takes wide character strings, not char based strings. In this case, the proper string type to use would be std::wstring.

You more than likely built an MBCS application, which in this day and age are becoming very rare.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
1
const char* addFoo(const char* strInput)
{
    return ("foo-" + std::string(strInput)).c_str();
}

This returns a pointer to a local temporary string, and its memory is released when your message box is shown.

Replace it by a std::string in your case:

std::string addFoo(const char* strInput)
{
    return std::string("foo-") + strInput; // not sure about the syntax here
}

Then, std::string object manages its memory correctly and will make the string pointer remain alive for long enough for the message box to display it. You'll need to include <string> to get this defined.

Then, you can use:

std::string temp = addFoo( a );
MessageBox(NULL, temp.c_str(), "The Foobar Experiment", MB_OK);
jpo38
  • 20,821
  • 10
  • 70
  • 151