Re: Problem with Vectors and STL/ATL in Visual C++ 2005
Colin J Paterson wrote:
The code is used in the _NewEnum function as follows
STDMETHODIMP CmxTables::get__NewEnum(LPUNKNOWN *pVal)
{
AFX_MANAGE_STATE(AfxGetStaticModuleState())
EnumVar *pEnumerator;
long iNumElements;
pEnumerator = new EnumVar;
iNumElements = m_Tables.GetSize();
pEnumerator->Init(m_Tables.GetAddress(0), m_Tables.GetAddress(iNumElements),
NULL, AtlFlagNoCopy);
if (FAILED(pEnumerator->QueryInterface(IID_IUnknown, (void**)pVal)))
goto cleanup;
return S_OK;
cleanup:
Not much of a cleanup routine, is it? How about:
delete pEnumerator;
return E_INVALIDARG;
}
If I make the changes you say I get a Automation Error in Visual Basic when
it returns the empty variant. The get address function seems to deliberately
return past the the end of the vector as described in the MSDN for
CComEnumImpl::Init.
Here is the function (m_bReverse is always false).
VARIANT* CmxArray::GetAddress(long iIndex)
{
// deliberate index error here
if(!m_bReverse)
{
if(IsEmpty() || (iIndex < 0) || iIndex >= m_vector.size())
{
m_emptyVariant.vt = VT_I2;
m_emptyVariant.iVal = -1;
return &m_emptyVariant;
}
return &m_vector[iIndex].m_Variant;
}
else
{
if (IsEmpty() || (iIndex < 0) || iIndex >= m_deque.size())
{
m_emptyVariant.vt = VT_I2;
m_emptyVariant.iVal = -1;
return &m_emptyVariant;
}
return &m_deque[iIndex].m_Variant;
}
}
I stress, this worked fine in VC6
I have to say that it is terrible code, and very surprising that it
"worked fine". It would have been better had it crashed, so you could
have fixed the bugs back when it was written.
and if I make a change to get__NewEnum to
look for (iNumElements - 1) then I lose the last element from every
vector????
Right. You have several separate problems with your code:
1. It calls m_vector[m_vector.size()], which is not legal, and on a nice
implementation will crash. On a nasty implementation, it will return
garbage (which is what is happening on VC6 - fortunately you only use
the address of the garbage, so the code survives).
2. The code uses the addresses into a vector (or deque) of
god-knows-whats in order to get a range of VARIANTs. e.g. if your vector
element type is this:
struct T
{
VARIANT m_Variant;
int foo;
};
it is never going to work, since if you do:
T t[2];
VARIANT* v = &t.m_Variant;
++v;
//here v points to foo, not a VARIANT
Even if your struct looks like this:
struct T
{
VARIANT m_Variant;
};
it is possible that the struct will have padding, in which case it still
won't work.
3. You are potentially (if m_bReverse is true) using two pointers into a
deque to form a range. Since deque is not contiguous, this is never
going to work unless the two pointers happen to lie in the same segment
of the deque.
So do you understand the problems? Now you need a solution. Well, the
hack to leave the code still incorrect, but apparently working is this:
VARIANT* CmxArray::GetAddress(long iIndex)
{
if (m_bReverse)
{
throw std::runtime_error(
"CmxArray::GetAddress not supported for reverse");
}
if(IsEmpty() || (iIndex < 0) || iIndex >= m_vector.size())
{
m_emptyVariant.vt = VT_I2;
m_emptyVariant.iVal = -1;
return &m_emptyVariant;
}
return &m_vector[0].m_Variant + iIndex;
}
To get it working properly, you need to remove this dodgy GetAddress
method entirely (which is never going to give you a range when working
with a deque), and instead use the appropriate class: CComEnumOnSTL.
Tom