A co-worker wrote the following code, which I am convinced is wrong.
I want to explain the problems to him, but don't know the proper term, so I cannot find references to support my position:
His code:
BSTR someString = _bstr_t(L"Hello World");
Why I think it is wrong:
I believe that _bstr_t(L"Hello World");
calls the constructor for _bstr_t
, and creates a short-lived temporary variable of that type. That temporary will be automatically deleted, and its string-space freed up, immediately after this line of code (after the semi-colon sequence point).
This will leave someString
pointing to memory that has been freed.
Questions:
What is the proper term for that call of a constructor?
Can you point to some references/terms/pages that describe the use in detail?
Is there a term for the temporary _bstr_t
object?
I guess I would call it an "anonymous, temporary variable", but I don't know if that is technically accurate.
(or maybe I'm completely wrong in my analysis.... if so, I'd love to know)
For clarification:
_bstr_t is a C++ class, commonly used by Microsoft to wrap their BSTR type, so it has constructors/destructors/operators, etc.
BSTR
is a typedef for just a WCHAR*
, so it does not have any logic. Its just a dumb-pointer.
You're right.
A BSTR
is a typedef for wchar_t *
, and CComBSTR/_bstr_t
has a non-const conversion operator to wchar_t *
.
Thus, a temporary _bstr_t
is allocated, a pointer to its beginning is assigned to someString
via the conversion operator, and the object is released as it goes out of scope. You then get a dangling pointer.
You can use
_bstr_t someString ("Hello World");
instead, or even _bstr_t someString = "Hello, World";
.
The code in question
BSTR someString = _bstr_t(L"Hello World");
is doing a conversion constructor call which creates a bstr_t
instance using the passed wchar_t[]
. This by itself is just fine. For example if you wanted to call a function accepting BSTR
and pass a string literal in there you could easily do this:
someFunction( _bstr_t(L"Hello World") ); // OKAY
and this would be okay because the temporary would survive until the end of the whole statement which ends where the semicolon resides (this is how C++ temporaries work).
However the code in question is not okay because the _bstr_t
instance is then used to instantiate a BSTR
instance (using a conversion operator in class bstr_t
) which lives longer than the temporary (the temporary is destroyed at the semicolon and the BSTR
pointer someString
lives well beyond that). So you get a dangling BSTR
pointer someString
using which leads to undefined behavior. It may even look working if the OLE heap chooses to keep the memory used for the string mapped into the process address space.
class _bstr_t
is supplied with the implementation (file comutil.h in Windows SDK) so you can step into the code with your debugger and see that there's one SysAllocString()
call when the temporary is created and then one SysFreeString()
call when it is destroyed and the latter happens before the code proceeds to the next line. Therefore the string object in the OLE heap is deallocated before the code proceeds to the next line and someString
pointer is dangling immediately after the code in question. I guess this would be enough to convince even the most skeptical person.
So yes, you're right that the code is erroneous. The correct code would be:
_bstr_t someString(L"Hello World");
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With