2

I have a very thorough understanding of memory and pointers, but I need a little refresher with regard to exactly how C++ manages some objects under the covers.

Consider the following code:

void Test()
{
    LPCTSTR psz = (LPCTSTR)GetString();
}

CString GetString()
{
    return CString(_T("abc"));
}

Questions:

  1. Could someone flesh out how GetString() can return a local object and it still be valid in the caller?

  2. Since the result of GetString() is not stored anywhere, how is it deleted?

  3. Is psz guaranteed to be "safe" to use for the entirety of the Test() function?

Sorry for using old classes for this example, but that's what I'm working with right now.

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • 1
    `LPCTSTR psz = (LPCTSTR)GetString();` That cast is completely wrong and obfuscates everything that's actually happening. It's undefined behavior, that's all. – πάντα ῥεῖ Feb 23 '17 at 18:22
  • @πάνταῥεῖ: And yet it compiles and runs just fine. So I'm looking for an explanation of why. – Jonathan Wood Feb 23 '17 at 18:23
  • 2
    @ πάντα Assuming this is the MFC CString class, I believe there is a user-defined conversion to LPCSTR. –  Feb 23 '17 at 18:23
  • @NeilButterworth: Yes, there absolutely is. – Jonathan Wood Feb 23 '17 at 18:24
  • 1
    _"And yet it compiles just fine."_ As a `reinterpret_cast` would also. You're sure you want to apply a `reinterpret_cast`? – πάντα ῥεῖ Feb 23 '17 at 18:24
  • @ πάντα It's not a reinterpret cast. –  Feb 23 '17 at 18:24
  • @JonathanWood Please clarify that in your question, and or tag it [tag:mfc]. – πάντα ῥεῖ Feb 23 '17 at 18:26
  • You get a pointer to the data portion of a temporary CString object, which is then immediately destructed. The memory might still contain the desired string for a little while, but there is no guarantee, especially if the function actually does anything. (or short form: UB, as someone already said) – Kenny Ostrom Feb 23 '17 at 18:28
  • @Jon Read up on copy constructors and RAII. –  Feb 23 '17 at 18:30
  • @FrançoisAndrieux: What the OP is asking in that question is about an obvious compiler error. There is no error here. – Jonathan Wood Feb 23 '17 at 18:32
  • @NeilButterworth: I'm very familiar with copy constructors. I wasn't familiar with the term RAII, but I'm didn't really see where it addressed this exact issue. – Jonathan Wood Feb 23 '17 at 18:35
  • It's not a compiler error in the other question. It is a warning, like your compiler should have given you here on this obviously wrong code. It is a duplicate in the sense that both retain a pointer to something which was destructed, and is therefore not valid to look at. – Kenny Ostrom Feb 23 '17 at 18:35
  • @KennyOstrom: AFAIC, it is never safe to do what the OP is doing in that other question. While my exact syntax may lead to issues, returning a `CString` is done all the time. – Jonathan Wood Feb 23 '17 at 18:36
  • @Jon Your point 1 is about copy construction, your point 2 about RAII. –  Feb 23 '17 at 18:37
  • @NeilButterworth: Okay, but I'm still not clear on the specifics of assigning one object to another when the object being assigned seems like it would have been destroyed (since the function has terminated). – Jonathan Wood Feb 23 '17 at 18:38
  • returning a CString is fine. You then extract its internal char*, destruct the CString, and then want to keep the char* from the internal of a destructed CString object. – Kenny Ostrom Feb 23 '17 at 18:38
  • 2
    **(1)** `GetString` returns a copy of a local object (though the actual copying may be elided, and the local temporary returned directly). **(2)** The result of `GetString()` is a temporary. Like most temporaries, it's automatically destroyed at the end of a full expression (essentially, at the semicolon). **(3)** `psz` gets a pointer to the buffer managed by that temporary. Once the temporary is destroyed, `psz` becomes dangling. Any attempt to actually use it would exhibit undefined behavior. – Igor Tandetnik Feb 23 '17 at 18:39

1 Answers1

7
  1. GetString returns a copy of a local object (though the actual copying may be elided, and the local temporary returned directly).

  2. The return value of GetString() is a temporary. Like most temporaries, it's automatically destroyed at the end of a full expression (essentially, at the semicolon).

  3. psz gets a pointer to the buffer managed by that temporary. Once the temporary is destroyed, psz becomes dangling. Any attempt to actually use it would exhibit undefined behavior.

Igor Tandetnik
  • 50,461
  • 4
  • 56
  • 85
  • Thanks. So it sounds like I should assign the result to a `CString` if I want to continue to work with it. But your second answer suggests I might be okay with expressions like `DoSomething((LPCTSTR)GetString());`. – Jonathan Wood Feb 23 '17 at 18:43
  • 2
    Yes, that's OK, as long as `DoSomething()` doesn't attempt to hang onto the pointer beyond the function call. – Igor Tandetnik Feb 23 '17 at 18:46
  • 2 points worth noting: Instead of the cast operator, it's generally a better idea to make an explicit function call, [CString::GetString()](https://msdn.microsoft.com/en-us/library/sddk80xf.aspx#csimplestringt__getstring) in this case. I believe you can also bind the temporary to a reference to const, which extends the temporary's lifetime to that of the reference, i.e. `const CString& s = GetString();` makes `s` valid throughout `void Test()`. – IInspectable Feb 23 '17 at 19:02
  • @IInspectable: Could you explain why `CString::GetString()` is better than a cast operator? – Jonathan Wood Feb 23 '17 at 19:16
  • 1
    @JonathanWood: If nothing else, it [thwarts off the know-it-alls](http://stackoverflow.com/questions/42423326/what-happens-when-a-function-returns-a-cstring#comment71992275_42423326). On a more serious note, user-defined cast operators are dangerous, because you cannot tell what's going on just by looking at the expression `(LPCTSTR)GetString()`. The function call is explicit, and can be immediately identified as such at the call site. No ambiguity introduced, no guessing required. – IInspectable Feb 23 '17 at 19:21
  • @IInspectable: So then, readability. As you can probably tell, I'm pretty old school, but I'm all for readability. :) – Jonathan Wood Feb 23 '17 at 19:22
  • 1
    @JonathanWood: Readability and maintainability. Should you ever decide to replace `CString` with another string class, the C-style cast can turn into a monster, whose only job it is to hide a compiler error. – IInspectable Feb 23 '17 at 19:26
  • `CString` has implicit conversion operators defined, so explicit typecasts are not needed: `DoSomething(GetString());` If you ever change `GetString()` to return something else, the code will likely fail to compile, and you can simply update the call site as needed. – Remy Lebeau Feb 23 '17 at 20:12