cout a returned string

Question

So I have my class called array and I want to return it as a formatted string, like this: [first, second, third, ..., last]. Now I wrote the method that is trying to do so:

std::string& array::to_string()
{
    char buffer[1];
    std::string s("[");
    for (auto &x: *this)
    {
        if (&x == this->end()) s += _itoa_s(x, buffer, 10) + "]";
        else s += _itoa_s(x, buffer, 10) + ",";
    }
    return s;
}

And yes, I've included <string>. Now, in the other part of my program I use std::cout << myArray.to_string() << '\n'. The error I get (during the execution) is just Visual Studio throwing me to stdlib.h header and showing this part of it:

__DEFINE_CPP_OVERLOAD_SECURE_FUNC_1_1(
    _Success_(return == 0)
    errno_t, _itoa_s,
    _In_ int,  _Value,
         char, _Buffer,
    _In_ int,  _Radix
    )

What am I doing wrong?

What is the error, and what line in your program does it come from? Step through your program with a debugger. Also, you probably need to show us more of your `array` class definition/implementation. — qxz, Mar 03 '17 at 16:16
Exactly from one of the lines from `if` statement (depending on the condition stated in if) — minecraftplayer1234, Mar 03 '17 at 16:17
In C++, you should probably use [`std::to_string`](http://stackoverflow.com/a/5591169/1848578) to convert numbers to `std::string`s — qxz, Mar 03 '17 at 16:19
I cannot use `std::` algorithms and containters. As the lecturer said, I can use `std::cout and std::string`, cuz I should focus on algorithms, when implementing containers. And the buffer is too small, but that isn't the issue, tried with `buffer[50]` — minecraftplayer1234, Mar 03 '17 at 16:27
@Frynio Your lecturer is contraddicting himself. std::string is a container. — roalz, Mar 03 '17 at 16:45
@roalz, I know, he also said that we have to do it in a language compiled into native code, so we can do it in `C\C++` but in `C#` as well, which isn't, by default, compiled this way — minecraftplayer1234, Mar 03 '17 at 16:48
@Frynio [facepalm] Poor guy! See my comment under my answer below... — roalz, Mar 03 '17 at 16:50

Tony Delroy · Accepted Answer · 2017-03-03T16:52:13.470

4

The string s is local to the function to_string, and its destructor runs as to_string returns, so returning and using a reference to the already-destructed string creates undefined behaviour. Return it by value instead:

std::string array::to_string() const
{
    // a more robust, C++-style implementation...
    std::ostringstream oss;
    size_t n = 0;
    for (const auto& x: *this)
        oss << (n++ ? ',' : '[') << x;
    oss << ']';
    return oss.str();
}

edited Mar 03 '17 at 16:52

answered Mar 03 '17 at 16:16

Tony Delroy

102,968
15
177
252

Did you pay attention to tkausl's feedback too? Separately, your loop will take you over the elements up to and excluding `end()`, which is *not* the last element but a sentinel value denoting "one-past-the-end" - so the `true` branch of your `if` statement is never executed. – Tony Delroy Mar 03 '17 at 16:31
`And the buffer is too small, but that isn't the issue, tried with buffer[50]` – minecraftplayer1234 Mar 03 '17 at 16:36
@Frynio: perhaps give the implementation I've edited into this answer a spin.... (`#include `). As for why yours still isn't working, you're using Microsoft's non-standard function `errno_t _itoa_s` - note the return type is an error number and not a `char*` suitable for adding to a `std::string`: you should be checking the return value then adding `buffer`. – Tony Delroy Mar 03 '17 at 16:40

roalz · Answer 2 · 2017-03-03T16:23:41.883

3

You are returning a reference to a std::string that is scoped inside your array::to_string() method.

When the method exits, the local s variable is out of scope, thus it is destroyed, resulting in a "dangling reference" being returned.

You should either return the string by value:

std::string array::to_string()

or pass it as a reference parameter:

void array::to_string(std::string& result)

edited Mar 03 '17 at 16:23

answered Mar 03 '17 at 16:18

roalz

2,699
3
25
42

Because you only have 1 character available in you "buffer" (char buffer[1];). Either use std::to_string(x) instead of that crappy buffer + _itoa_s(), or (due to your constraints) enlarge your buffer[] – roalz Mar 03 '17 at 16:47

score 2 · Answer 3 · answered Mar 03 '17 at 16:17

2

You are returning a reference to a local object. Because of this, you try to use an object that has been deallocated from the stack. Remove the & from the function signature to return it by value.

answered Mar 03 '17 at 16:17

Rosme

1,049
9
23

Still the same problem – minecraftplayer1234 Mar 03 '17 at 16:29

cout a returned string

3 Answers3