CODEWITHSUNDEEP
c++undefined-behaviorconditional-operator
Nuclear
Nuclear

Reputation: 1458

Returning std::string via ternary operator - returns garbage

I'm having the following code:

#include <string>
#include <cstdio>

std::string name = "Ternary Return Test";
std::string *pname = &name;

const std::string &getName ()
{
    return pname ? *pname : "(unnamed)";
}

int main (int argc, char *argv[])
{
    const std::string &str = getName();
    printf ("Name is \"%s\"\n",str.c_str());
    printf ("pName is \"%s\"\n",pname->c_str());
    return 0; 
}

The function getName() dereferences the pname pointer and returns it's value as a reference. As I understand, returning reference to something, through dereferencing of a pointer is perfectly valid. In order to avoid NULL pointer dereferencing, the function returns the string "unnamed", in the (in this situation impossible) case, when the pname pointer is NULL.

However, the ternary operator breaks the function. The code compiles, but prints following:

Name is "�
          @"
pName is "Ternary Return Test"

The value, obtained through the function is broken. I cannot understand why. In other situation, similar code produces segmentation fault. So, an undefined behaviour is happening here?

Yes, in the case when the pname would be NULL, it would return a reference to temporary, which is undefined. But this is not the case - I am returning a reference to a non-temporary value, which should be perfectly valid.

As I read, the ternary operator converts the third expression "(unnamed)" to the type of the second (std::string). But in my case, the third expression is not even used.

Upvotes: 0

Views: 697

Answers (4)

Useless
Useless

Reputation: 67733

For reference (the dangling-reference UB is well described already), this should probably be something like

const std::string &getName ()
{
    static const std::string unnamed("unnamed");
    return pname ? *pname : unnamed;
}

Upvotes: 0

Jarod42
Jarod42

Reputation: 217265

ternary operator type for

pname ? *pname : "(unnamed)" is std::string (we have std::string& and const char*)

So it is equivalent to pname ? std::string{*pname} : std::string{"(unnamed)"}.

So you return reference from temporary variable (in both cases) (and so dangling reference, leading to UB when you use it).

If you have used if instead of ternary, you would have your expected error

const std::string& getName()
{
    if pname { return *pname; } // OK here
    return "(unnamed)"; // Dangling pointer here
}

One solution might be;

const std::string &getName ()
{
    static const std::string unnamed = "(unnamed)";
    return pname ? *pname : unnamed;
}

So both sides are lvalue, and common type is now const std::string & (std::string & and const std::string&).

Upvotes: 3

MSalters
MSalters

Reputation: 179819

As Yksisarvinen points out, the problem is a lifetime issue. "(unnamed)" has static storage duration, but that's not what you return. You return std::string{"(unnamed)"}, and a reference to be exact. That unnamed temporary goes away before the function even returns.

Fix: 

std::string const& getName ()
{
    static const std::string defaultValue = "(unnamed)";
    return pname ? *pname : defaultValue;
}

Upvotes: 1

NathanOliver
NathanOliver

Reputation: 180585

But this is not the case - I am returning a reference to a non-temporary value, which should be perfectly valid.

You would be, but you don't have a non-temporary value anymore. [expr.cond]/4.3 has

If E2 is a prvalue or if neither of the conversion sequences above can be formed and at least one of the operands has (possibly cv-qualified) class type:

  • if T1 and T2 are the same class type (ignoring cv-qualification) and T2 is at least as cv-qualified as T1, the target type is T2,

And what that boils down to is that since the third operand is a prvalue, the entire expression is a prvalue which mean you always have undefined behavior as you are always returning a reference to a temporary.

Upvotes: 4

Related Questions