Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What is the terminology for this use of a constructor?

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.

like image 896
abelenky Avatar asked Jul 03 '14 19:07

abelenky


2 Answers

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";.

like image 65
Alexandre C. Avatar answered Nov 15 '22 03:11

Alexandre C.


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");
like image 28
sharptooth Avatar answered Nov 15 '22 04:11

sharptooth