return string with persistence buffer

Question

I tried to write a function that formats a value into string. However, i encounter issue with its persistence.

using namespace std;

string string_format(const string &msg, ...)
{
    va_list ap;
    char text[BUFF_SIZE] = { 0, };
    va_start(ap, msg);
    vsnprintf(text, BUFF_SIZE - 1, msg.c_str(), ap);
    text[BUFF_SIZE - 1] = '\0';
    va_end(ap);

    return string(text);
}

int main()
{
    char* p1 = (char*)string_format("%d", 123).c_str();
    char* p2 = (char*)string_format("%d", 45).c_str();

    printf("value: p1=%s, p2=%s\r\n", p1, p2);
}

The above will print:

value: p1=45, p2=45

instead of

value: p1=123, p2=45

I wrote the code this way because i want to keep 2 or more char* instance at the same time. return "new string(text)" works, but i dont want to keep writing "delete"

possible duplicate of [Easiest way to convert int to string in C++](http://stackoverflow.com/questions/5590381/easiest-way-to-convert-int-to-string-in-c) — Cool_Coder, May 05 '14 at 02:33
the format does not necessarily to be int, it can be char* p1 = (char*)string_format("%s", "hello").c_str(); char* p2 = (char*)string_format("%s", "world").c_str(); in which it will print: value: p1=world, p2=world — Va1iant, May 05 '14 at 02:49
i was trying to make my code more readable. since there are a lot of string need to be formatted, i was thinking of a function to create one line formatting + assigning: `stA.buff1 = string_format("%s", "how").c_str();` `stA.buff2 = string_format("%s", "are").c_str();` `...` — Va1iant, May 05 '14 at 03:52
@Va1iant it causes undefined behaviour to have a reference type as the last parameter before the `...` . You need to either take the string by value, or take `char const *`. The fact that it appears to work currently is no guarantee that it'll keep working as you add more things to your program. — M.M, May 05 '14 at 04:23
@Brandon, this version could be improved to allocate the right amount of space; unlike `sprintf`. (There are some non-standard self-allocating versions of `sprintf` around, of course) — M.M, May 05 '14 at 04:25
@MattMcNabb; EDIT: Oh that semi-colon makes all the difference. I see what you mean now. — Brandon, May 05 '14 at 04:34
A very similar problem in another SO post: http://stackoverflow.com/questions/23464504/string-and-const-char-and-c-str/23464576#23464576 — R Sahu, May 05 '14 at 04:56

score 2 · Answer 1 · answered May 05 '14 at 02:31

When you use c_str() you're getting a pointer to an internal buffer in the string. That pointer is only valid as long as the string object is still alive. You're taking a pointer to a temporary string object that's immediately getting deleted, which causes the incorrect behavior. The reason it manifests as "persistence" is probably because the two very similar strings just happen to use the same memory space. Accessing the pointers after the string has been destroyed is undefined behavior in any case.

If you want to use c_str, you need to store your resulting strings into proper string variables and keep them there for as long as you need the pointer.

Or even better, don't deal with char * at all. Since you're using C++, use proper string objects for your strings always.

score 2 · Answer 2 · edited May 05 '14 at 04:21

2

Store the results in a string and use .c_str() when you need.

string p1 = string_format("%d", 123);
string p2 = string_format("%d", 45);

printf("value: p1=%s, p2=%s\r\n", p1.c_str(), p2.c_str());

edited May 05 '14 at 04:21

M.M

138,810
21
208
365

answered May 05 '14 at 02:31

The Dark

8,453
1
16
19

Va1iant · Answer 3 · 2014-05-12T16:42:10.427

0

thanks for the quick and clear response. To achieve my original objective (make my code more readable, since this kind of buffer assignments are common in the module i'm writing), I ended up using a macro that declare a string as a temporary placeholder. This time it shows the correct result

#define ASSIGN_BUFFER(var, format, value) \
    string str_##var = string_format(format, value) \
    var = (char*)str_##var.c_val()

string string_format(const string &msg, ...)
{
    va_list ap;
    char text[BUFF_SIZE] = { 0, };
    va_start(ap, msg);
    vsnprintf(text, BUFF_SIZE - 1, msg.c_str(), ap);
    text[BUFF_SIZE - 1] = '\0';
    va_end(ap);

    return string(text);
}

int main()
{
    char *p1, *p2;

    ASSIGN_BUFFER(p1, "%d", 123);
    ASSIGN_BUFFER(p2, "%d", 45);

    printf("value: p1=%s, p2=%s\r\n", p1, p2);
}

edited May 12 '14 at 16:42

answered May 09 '14 at 20:44

Va1iant

203
3
12

This still has the problem pointed out by Matt: “it causes undefined behaviour to have a reference type as the last parameter before the `...` ” – Konrad Rudolph May 12 '14 at 16:51
could you explained the reason of the undefined behavior? and why it would be different if i put string/const char* instead – Va1iant May 12 '14 at 20:12
Unfortunately, the only explanation is: because the standard says so. It’s defined in §18.10/3 but no explanation is given. The most likely explanation is that this is a C language mechanism which only exists in C++ for reasons of backwards compatibility. The proper C++ way would be to use [variadic templates](https://en.wikipedia.org/wiki/Variadic_template), which is better anyway. – Konrad Rudolph May 12 '14 at 20:25
that's fair enough. Actually i put the parameter as "const string &msg" because i saw it from another stackoverflow thread. But i guess if the standard say so, better to comply than sorry. Thanks for bringing out the concern i would otherwise overlooked – Va1iant May 13 '14 at 17:07

return string with persistence buffer

3 Answers3