Undefined behavior when converting char to std::string c++

Question

My program takes in a vector std::vector<std::string> vector and a character char separator and returns a string with all of the strings added together between the separator character. The concept is: vector[0] + separator + vector[1] + separator

The Code

std::string VectorToString(std::vector<std::string> vector, char separator)
{
    std::string output;
    for(std::string segment : vector)
    {  
        std::string separator_string(&separator);
        output += segment + separator_string;
    }
    return output;
}

int main()
{
    std::vector<std::string> vector = {"Hello",  "my",  "beautiful", "people"};
    std::cout << VectorToString(vector, ' ');
}

My expected output is Hello my beautiful people However the output is:

Hello ��my ��beautiful ��people ��

What I have found is that something is wrong with the character, specifically its pointer: std::cout << &separator; -> �ƚ��. However if I do like this: std::cout << (void*) &separator; -> 0x7ffee16d35f7. Though I don't really know what (void*) does.

Question:

1.What is happening?

2.Why is it happening?

3.How do I fix it?

4.How do I prevent it from happening in future projekts?

[casting - C++ convert from 1 char to string? - Stack Overflow](https://stackoverflow.com/questions/17201590/c-convert-from-1-char-to-string) — user202729, Feb 06 '21 at 08:27
...That doesn't contain explanation on why it's undefined behavior... — user202729, Feb 06 '21 at 08:28
This doesn't address the question, but creating that `separator_string` object every time through the loop wastes time. Create it once, before the loop. Or, even better, don't create it at all. Just append the character to the string: `output += segment; output += separator;`. (Yes, string objects know how to append a single character) And for the loop, create a reference to each string rather than copying: `for (const std::string& segment : vector)`. — Pete Becker, Feb 06 '21 at 14:35
@PeteBecker why should I use a reference instead of copying. Is it faster or are there any other reasons? — Viktor Skarve, Feb 07 '21 at 09:04
@ViktorSkarve -- it's faster, and uses fewer resources. For simple programs like this one that doesn't really matter, but when you get into more complex programs it can make a big difference, both in speed and in memory usage. It can make the difference between running successfully and running out of memory. In general, it's a good habit to use `const` references rather than copying big things. That's why so many functions take `const std::string&`. — Pete Becker, Feb 07 '21 at 14:53

StPiere · Accepted Answer · 2021-02-06T08:52:47.230

4

This line

std::string separator_string(&separator);

tries to construct a string from a 0-terminated C string.

But &separator is not 0-terminated, because it depends on the other bytes of memory following separator if there is an 0 byte in there (probably not). So you're getting undefined behavior.

What you can do is to use other constructor:

std::string separator_string(1, separator);

This one creates a string by repeating separator character 1 time.

edited Feb 06 '21 at 08:52

answered Feb 06 '21 at 08:27

StPiere

4,113
15
24

So std::string separator_string(&separator); should be used with c-string and not char? – Viktor Skarve Feb 06 '21 at 08:35
1

yeah,because for ex. this one would work: separator_string(" "); because " " is a zero terminated C string. – StPiere Feb 06 '21 at 08:39
2

+1, I totally didn't see the `&`. Btw, you don't need to construct a string first, `std::string` overloads `operator+` for char too. So `output += segment + separator;` should be enough. – Lukas-T Feb 06 '21 at 08:39

score 1 · Answer 2 · answered Feb 06 '21 at 08:44

By the standard, following:

basic_string( const CharT* s,
              const Allocator& alloc = Allocator() );

means this:

Constructs the string with the contents initialized with a copy of the null-terminated character string pointed to by s. The length of the string is determined by the first null character. The behavior is undefined if [s, s + Traits::length(s)) is not a valid range.

Therefore, std::string separator_string(&separator); causes an Undefined-Behavior because separator is not null-terminated.

To prevent this, you might want to use the following overload:

basic_string( const CharT* s,
              size_type count,
              const Allocator& alloc = Allocator() );

like std::string separator_string(&separator, 1); or even more simpler (as other answer pointed out) std::string separator_string(1, separator);.

Undefined behavior when converting char to std::string c++

2 Answers2