_variant_t operator= (const wchar_t*) 
Author Message
 _variant_t operator= (const wchar_t*)

I believe there is a bug in the first line in the following method, found at
line 1743 of comutil.h for VC.Net

----------

inline _variant_t& _variant_t::operator=(const wchar_t* pSrc)
throw(_com_error)
{
    // Clear the VARIANT (This will SysFreeString() any previous occupant)
    //
    _COM_ASSERT(V_BSTR(this) != pSrc);                      <===  bug is
here -- uses bstrval even if currently cleared

...

----------

The problem here is that it appears that ::VariantClear does not null the
bstr pointers, so they cannot be relied on to be null for non-bstrs.

This creates an issue when the bstr being assigned was detached from the
variant and later reassigned.

The line is new for VC.NET -- it was not there for VC6.

Am I missing something obvious, or is this a bug?

- Gardner



Wed, 15 Dec 2004 00:08:27 GMT  
 _variant_t operator= (const wchar_t*)
I have this same problem. If the variant is empty and you assign a
_bstr_t to it, you get this error. To get around it I just use a
constructor when I do an assignment. I hope the optimizer is smart
enough to get rid of it:

   _variant_t vtTest;
   _bstr_t btTest;

   btTest = L"1";
   vtTest = btTest;   // causes the assert
   vtTest = _variant_t(btTest);   // OK

It also comes up if you have code that happens to reuse a variant and
assigns the same value to it (of any type). I think Microsoft is
saying: "You are dumb to be assigning the same value to a variant that
is already in there."

I think it is a waste of processing time to keep constructing
_variant_t's on the stack if you don't need to. It is definitely
annoying.

I couldn't find anything in the Microsoft Knowledge base about this.

Ken
==========================================


Quote:
> I believe there is a bug in the first line in the following method, found at
> line 1743 of comutil.h for VC.NET

> ----------

> inline _variant_t& _variant_t::operator=(const wchar_t* pSrc)
> throw(_com_error)
> {
>     // Clear the VARIANT (This will SysFreeString() any previous occupant)
>     //
>     _COM_ASSERT(V_BSTR(this) != pSrc);                      <===  bug is
> here -- uses bstrval even if currently cleared

> ...

> ----------

> The problem here is that it appears that ::VariantClear does not null the
> bstr pointers, so they cannot be relied on to be null for non-bstrs.

> This creates an issue when the bstr being assigned was detached from the
> variant and later reassigned.

> The line is new for VC.NET -- it was not there for VC6.

> Am I missing something obvious, or is this a bug?

> - Gardner



Tue, 04 Jan 2005 06:33:41 GMT  
 _variant_t operator= (const wchar_t*)
I've hit two other bugs like this in comutil.h.  The first is here:

inline _variant_t& _variant_t::operator=(const _bstr_t& bstrSrc)
throw(_com_error)
{
    // Clear the VARIANT (This will SysFreeString() any previous
occupant)
    //
    _COM_ASSERT(V_BSTR(this) != (BSTR) bstrSrc);

We often hit this in loops.  Let's say you have a _variant_t on the
stack and you loop, setting it to a value returned from a function.
If you happen to return the same _bstr_t twice in a row you'll hit the
assert.  The destructor/constructor of the variant does not clear the
bstrVal field so it's left with the previous value.

Second is here:

// Assign a const wchar_t* to a _bstr_t
//
inline _bstr_t& _bstr_t::operator=(const wchar_t* s) throw(_com_error)
{
    _COM_ASSERT((const wchar_t*) *this != s);

Let's say you have a _bstr_t that is empty (it's never been assigned
to).  Then you assign to it from another empty string (it will also be
NULL).  You'll hit this assert.

Attempting to change our code to fix this would be unreasonable.  We
have many, many assignments to _variant_t and _bstr_t values and can't
check each time for these cases (it would also be very expensive at
run time).  I'm surprised I haven't seen these bugs in a knowledge
base article.  Right now our plan is to hack comutil.h on all
developer's machines.  Is there any other work around?  Do these
asserts truly indicate something wrong with our code?

-- derek

Quote:

> I have this same problem. If the variant is empty and you assign a
> _bstr_t to it, you get this error. To get around it I just use a
> constructor when I do an assignment. I hope the optimizer is smart
> enough to get rid of it:

>    _variant_t vtTest;
>    _bstr_t btTest;

>    btTest = L"1";
>    vtTest = btTest;   // causes the assert
>    vtTest = _variant_t(btTest);   // OK

> It also comes up if you have code that happens to reuse a variant and
> assigns the same value to it (of any type). I think Microsoft is
> saying: "You are dumb to be assigning the same value to a variant that
> is already in there."

> I think it is a waste of processing time to keep constructing
> _variant_t's on the stack if you don't need to. It is definitely
> annoying.

> I couldn't find anything in the Microsoft Knowledge base about this.

> Ken
> ==========================================


> > I believe there is a bug in the first line in the following method, found at
> > line 1743 of comutil.h for VC.NET

> > ----------

> > inline _variant_t& _variant_t::operator=(const wchar_t* pSrc)
> > throw(_com_error)
> > {
> >     // Clear the VARIANT (This will SysFreeString() any previous occupant)
> >     //
> >     _COM_ASSERT(V_BSTR(this) != pSrc);                      <===  bug is
> > here -- uses bstrval even if currently cleared

> > ...

> > ----------

> > The problem here is that it appears that ::VariantClear does not null the
> > bstr pointers, so they cannot be relied on to be null for non-bstrs.

> > This creates an issue when the bstr being assigned was detached from the
> > variant and later reassigned.

> > The line is new for VC.NET -- it was not there for VC6.

> > Am I missing something obvious, or is this a bug?

> > - Gardner



Sun, 16 Jan 2005 01:42:45 GMT  
 _variant_t operator= (const wchar_t*)
Derek,

Thanks for the pointer to the additional cases.  I missed them.

I was kind of hoping that someone from [MS] would at least comment on things
marked [BUG].   Perhaps I will later if noone from [MS] comments.

So far we have been lucky in that by writing a BetterVariantClear method,
and calling it instead of Variant Clear, we have been able to avoid all
instances of this as far as we can tell.

- Gardner


Quote:
> I've hit two other bugs like this in comutil.h.  The first is here:

> inline _variant_t& _variant_t::operator=(const _bstr_t& bstrSrc)
> throw(_com_error)
> {
>     // Clear the VARIANT (This will SysFreeString() any previous
> occupant)
>     //
>     _COM_ASSERT(V_BSTR(this) != (BSTR) bstrSrc);

> We often hit this in loops.  Let's say you have a _variant_t on the
> stack and you loop, setting it to a value returned from a function.
> If you happen to return the same _bstr_t twice in a row you'll hit the
> assert.  The destructor/constructor of the variant does not clear the
> bstrVal field so it's left with the previous value.

> Second is here:

> // Assign a const wchar_t* to a _bstr_t
> //
> inline _bstr_t& _bstr_t::operator=(const wchar_t* s) throw(_com_error)
> {
>     _COM_ASSERT((const wchar_t*) *this != s);

> Let's say you have a _bstr_t that is empty (it's never been assigned
> to).  Then you assign to it from another empty string (it will also be
> NULL).  You'll hit this assert.

> Attempting to change our code to fix this would be unreasonable.  We
> have many, many assignments to _variant_t and _bstr_t values and can't
> check each time for these cases (it would also be very expensive at
> run time).  I'm surprised I haven't seen these bugs in a knowledge
> base article.  Right now our plan is to hack comutil.h on all
> developer's machines.  Is there any other work around?  Do these
> asserts truly indicate something wrong with our code?

> -- derek




- Show quoted text -

Quote:
> > I have this same problem. If the variant is empty and you assign a
> > _bstr_t to it, you get this error. To get around it I just use a
> > constructor when I do an assignment. I hope the optimizer is smart
> > enough to get rid of it:

> >    _variant_t vtTest;
> >    _bstr_t btTest;

> >    btTest = L"1";
> >    vtTest = btTest;   // causes the assert
> >    vtTest = _variant_t(btTest);   // OK

> > It also comes up if you have code that happens to reuse a variant and
> > assigns the same value to it (of any type). I think Microsoft is
> > saying: "You are dumb to be assigning the same value to a variant that
> > is already in there."

> > I think it is a waste of processing time to keep constructing
> > _variant_t's on the stack if you don't need to. It is definitely
> > annoying.

> > I couldn't find anything in the Microsoft Knowledge base about this.

> > Ken
> > ==========================================




- Show quoted text -

Quote:
> > > I believe there is a bug in the first line in the following method,
found at
> > > line 1743 of comutil.h for VC.NET

> > > ----------

> > > inline _variant_t& _variant_t::operator=(const wchar_t* pSrc)
> > > throw(_com_error)
> > > {
> > >     // Clear the VARIANT (This will SysFreeString() any previous
occupant)
> > >     //
> > >     _COM_ASSERT(V_BSTR(this) != pSrc);                      <===  bug
is
> > > here -- uses bstrval even if currently cleared

> > > ...

> > > ----------

> > > The problem here is that it appears that ::VariantClear does not null
the
> > > bstr pointers, so they cannot be relied on to be null for non-bstrs.

> > > This creates an issue when the bstr being assigned was detached from
the
> > > variant and later reassigned.

> > > The line is new for VC.NET -- it was not there for VC6.

> > > Am I missing something obvious, or is this a bug?

> > > - Gardner



Sun, 16 Jan 2005 12:02:35 GMT  
 _variant_t operator= (const wchar_t*)
Could someone from Microsoft validate that the bugs metnioned in this recent
conversation are known bugs?

Thanks in advance,

Gardner

Quote:

> Derek,

> Thanks for the pointer to the additional cases.  I missed them.

> I was kind of hoping that someone from [MS] would at least comment on
things
> marked [BUG].   Perhaps I will later if noone from [MS] comments.

> So far we have been lucky in that by writing a BetterVariantClear method,
> and calling it instead of Variant Clear, we have been able to avoid all
> instances of this as far as we can tell.

> - Gardner



> > I've hit two other bugs like this in comutil.h.  The first is here:

> > inline _variant_t& _variant_t::operator=(const _bstr_t& bstrSrc)
> > throw(_com_error)
> > {
> >     // Clear the VARIANT (This will SysFreeString() any previous
> > occupant)
> >     //
> >     _COM_ASSERT(V_BSTR(this) != (BSTR) bstrSrc);

> > We often hit this in loops.  Let's say you have a _variant_t on the
> > stack and you loop, setting it to a value returned from a function.
> > If you happen to return the same _bstr_t twice in a row you'll hit the
> > assert.  The destructor/constructor of the variant does not clear the
> > bstrVal field so it's left with the previous value.

> > Second is here:

> > // Assign a const wchar_t* to a _bstr_t
> > //
> > inline _bstr_t& _bstr_t::operator=(const wchar_t* s) throw(_com_error)
> > {
> >     _COM_ASSERT((const wchar_t*) *this != s);

> > Let's say you have a _bstr_t that is empty (it's never been assigned
> > to).  Then you assign to it from another empty string (it will also be
> > NULL).  You'll hit this assert.

> > Attempting to change our code to fix this would be unreasonable.  We
> > have many, many assignments to _variant_t and _bstr_t values and can't
> > check each time for these cases (it would also be very expensive at
> > run time).  I'm surprised I haven't seen these bugs in a knowledge
> > base article.  Right now our plan is to hack comutil.h on all
> > developer's machines.  Is there any other work around?  Do these
> > asserts truly indicate something wrong with our code?

> > -- derek



> > > I have this same problem. If the variant is empty and you assign a
> > > _bstr_t to it, you get this error. To get around it I just use a
> > > constructor when I do an assignment. I hope the optimizer is smart
> > > enough to get rid of it:

> > >    _variant_t vtTest;
> > >    _bstr_t btTest;

> > >    btTest = L"1";
> > >    vtTest = btTest;   // causes the assert
> > >    vtTest = _variant_t(btTest);   // OK

> > > It also comes up if you have code that happens to reuse a variant and
> > > assigns the same value to it (of any type). I think Microsoft is
> > > saying: "You are dumb to be assigning the same value to a variant that
> > > is already in there."

> > > I think it is a waste of processing time to keep constructing
> > > _variant_t's on the stack if you don't need to. It is definitely
> > > annoying.

> > > I couldn't find anything in the Microsoft Knowledge base about this.

> > > Ken
> > > ==========================================



> > > > I believe there is a bug in the first line in the following method,
> found at
> > > > line 1743 of comutil.h for VC.NET

> > > > ----------

> > > > inline _variant_t& _variant_t::operator=(const wchar_t* pSrc)
> > > > throw(_com_error)
> > > > {
> > > >     // Clear the VARIANT (This will SysFreeString() any previous
> occupant)
> > > >     //
> > > >     _COM_ASSERT(V_BSTR(this) != pSrc);                      <===
bug
> is
> > > > here -- uses bstrval even if currently cleared

> > > > ...

> > > > ----------

> > > > The problem here is that it appears that ::VariantClear does not
null
> the
> > > > bstr pointers, so they cannot be relied on to be null for non-bstrs.

> > > > This creates an issue when the bstr being assigned was detached from
> the
> > > > variant and later reassigned.

> > > > The line is new for VC.NET -- it was not there for VC6.

> > > > Am I missing something obvious, or is this a bug?

> > > > - Gardner



Sat, 22 Jan 2005 03:23:09 GMT  
 _variant_t operator= (const wchar_t*)
Yes.

As a temporary workaround you can certainly add a null check to these
asserts such as
    _COM_ASSERT(V_BSTR(this) != (BSTR) bstrSrc);
to
    _COM_ASSERT(bstrSrc==0 || V_BSTR(this) != (BSTR) bstrSrc);

We will have this fixed in an upcoming version of VC++.



Quote:
> Could someone from Microsoft validate that the bugs metnioned in this
recent
> conversation are known bugs?

> Thanks in advance,

> Gardner


> > Derek,

> > Thanks for the pointer to the additional cases.  I missed them.

> > I was kind of hoping that someone from [MS] would at least comment on
> things
> > marked [BUG].   Perhaps I will later if noone from [MS] comments.

> > So far we have been lucky in that by writing a BetterVariantClear
method,
> > and calling it instead of Variant Clear, we have been able to avoid all
> > instances of this as far as we can tell.

> > - Gardner



> > > I've hit two other bugs like this in comutil.h.  The first is here:

> > > inline _variant_t& _variant_t::operator=(const _bstr_t& bstrSrc)
> > > throw(_com_error)
> > > {
> > >     // Clear the VARIANT (This will SysFreeString() any previous
> > > occupant)
> > >     //
> > >     _COM_ASSERT(V_BSTR(this) != (BSTR) bstrSrc);

> > > We often hit this in loops.  Let's say you have a _variant_t on the
> > > stack and you loop, setting it to a value returned from a function.
> > > If you happen to return the same _bstr_t twice in a row you'll hit the
> > > assert.  The destructor/constructor of the variant does not clear the
> > > bstrVal field so it's left with the previous value.

> > > Second is here:

> > > // Assign a const wchar_t* to a _bstr_t
> > > //
> > > inline _bstr_t& _bstr_t::operator=(const wchar_t* s) throw(_com_error)
> > > {
> > >     _COM_ASSERT((const wchar_t*) *this != s);

> > > Let's say you have a _bstr_t that is empty (it's never been assigned
> > > to).  Then you assign to it from another empty string (it will also be
> > > NULL).  You'll hit this assert.

> > > Attempting to change our code to fix this would be unreasonable.  We
> > > have many, many assignments to _variant_t and _bstr_t values and can't
> > > check each time for these cases (it would also be very expensive at
> > > run time).  I'm surprised I haven't seen these bugs in a knowledge
> > > base article.  Right now our plan is to hack comutil.h on all
> > > developer's machines.  Is there any other work around?  Do these
> > > asserts truly indicate something wrong with our code?

> > > -- derek



> > > > I have this same problem. If the variant is empty and you assign a
> > > > _bstr_t to it, you get this error. To get around it I just use a
> > > > constructor when I do an assignment. I hope the optimizer is smart
> > > > enough to get rid of it:

> > > >    _variant_t vtTest;
> > > >    _bstr_t btTest;

> > > >    btTest = L"1";
> > > >    vtTest = btTest;   // causes the assert
> > > >    vtTest = _variant_t(btTest);   // OK

> > > > It also comes up if you have code that happens to reuse a variant
and
> > > > assigns the same value to it (of any type). I think Microsoft is
> > > > saying: "You are dumb to be assigning the same value to a variant
that
> > > > is already in there."

> > > > I think it is a waste of processing time to keep constructing
> > > > _variant_t's on the stack if you don't need to. It is definitely
> > > > annoying.

> > > > I couldn't find anything in the Microsoft Knowledge base about this.

> > > > Ken
> > > > ==========================================



> > > > > I believe there is a bug in the first line in the following
method,
> > found at
> > > > > line 1743 of comutil.h for VC.NET

> > > > > ----------

> > > > > inline _variant_t& _variant_t::operator=(const wchar_t* pSrc)
> > > > > throw(_com_error)
> > > > > {
> > > > >     // Clear the VARIANT (This will SysFreeString() any previous
> > occupant)
> > > > >     //
> > > > >     _COM_ASSERT(V_BSTR(this) != pSrc);                      <===
> bug
> > is
> > > > > here -- uses bstrval even if currently cleared

> > > > > ...

> > > > > ----------

> > > > > The problem here is that it appears that ::VariantClear does not
> null
> > the
> > > > > bstr pointers, so they cannot be relied on to be null for
non-bstrs.

> > > > > This creates an issue when the bstr being assigned was detached
from
> > the
> > > > > variant and later reassigned.

> > > > > The line is new for VC.NET -- it was not there for VC6.

> > > > > Am I missing something obvious, or is this a bug?

> > > > > - Gardner



Wed, 26 Jan 2005 14:55:15 GMT  
 _variant_t operator= (const wchar_t*)
Hi Jeff,

Thanks for seeing and commenting on this.

But, I must point out that what you propose is not a workaround for the
original case I posted -- please note the bug at the bottom of this thread
is triggered by a matching address in the variant, even if the variant is
already cleared.  Your test would also need to not fire if the current
variant type is not VT_BSTR.

We are seeing this where the variant has been cleared using
_variant_t::Clear, and the new assignment happens be from the same address
as the prior value of the variant's bstr.  This is due to the variant clear
not clearing out the bstr pointer, yet this test is using it without
checking the variant type

- Gardner



Quote:
> Yes.

> As a temporary workaround you can certainly add a null check to these
> asserts such as
>     _COM_ASSERT(V_BSTR(this) != (BSTR) bstrSrc);
> to
>     _COM_ASSERT(bstrSrc==0 || V_BSTR(this) != (BSTR) bstrSrc);

> We will have this fixed in an upcoming version of VC++.



> > Could someone from Microsoft validate that the bugs metnioned in this
> recent
> > conversation are known bugs?

> > Thanks in advance,

> > Gardner


> > > Derek,

> > > Thanks for the pointer to the additional cases.  I missed them.

> > > I was kind of hoping that someone from [MS] would at least comment on
> > things
> > > marked [BUG].   Perhaps I will later if noone from [MS] comments.

> > > So far we have been lucky in that by writing a BetterVariantClear
> method,
> > > and calling it instead of Variant Clear, we have been able to avoid
all
> > > instances of this as far as we can tell.

> > > - Gardner



> > > > I've hit two other bugs like this in comutil.h.  The first is here:

> > > > inline _variant_t& _variant_t::operator=(const _bstr_t& bstrSrc)
> > > > throw(_com_error)
> > > > {
> > > >     // Clear the VARIANT (This will SysFreeString() any previous
> > > > occupant)
> > > >     //
> > > >     _COM_ASSERT(V_BSTR(this) != (BSTR) bstrSrc);

> > > > We often hit this in loops.  Let's say you have a _variant_t on the
> > > > stack and you loop, setting it to a value returned from a function.
> > > > If you happen to return the same _bstr_t twice in a row you'll hit
the
> > > > assert.  The destructor/constructor of the variant does not clear
the
> > > > bstrVal field so it's left with the previous value.

> > > > Second is here:

> > > > // Assign a const wchar_t* to a _bstr_t
> > > > //
> > > > inline _bstr_t& _bstr_t::operator=(const wchar_t* s)
throw(_com_error)
> > > > {
> > > >     _COM_ASSERT((const wchar_t*) *this != s);

> > > > Let's say you have a _bstr_t that is empty (it's never been assigned
> > > > to).  Then you assign to it from another empty string (it will also
be
> > > > NULL).  You'll hit this assert.

> > > > Attempting to change our code to fix this would be unreasonable.  We
> > > > have many, many assignments to _variant_t and _bstr_t values and
can't
> > > > check each time for these cases (it would also be very expensive at
> > > > run time).  I'm surprised I haven't seen these bugs in a knowledge
> > > > base article.  Right now our plan is to hack comutil.h on all
> > > > developer's machines.  Is there any other work around?  Do these
> > > > asserts truly indicate something wrong with our code?

> > > > -- derek



> > > > > I have this same problem. If the variant is empty and you assign a
> > > > > _bstr_t to it, you get this error. To get around it I just use a
> > > > > constructor when I do an assignment. I hope the optimizer is smart
> > > > > enough to get rid of it:

> > > > >    _variant_t vtTest;
> > > > >    _bstr_t btTest;

> > > > >    btTest = L"1";
> > > > >    vtTest = btTest;   // causes the assert
> > > > >    vtTest = _variant_t(btTest);   // OK

> > > > > It also comes up if you have code that happens to reuse a variant
> and
> > > > > assigns the same value to it (of any type). I think Microsoft is
> > > > > saying: "You are dumb to be assigning the same value to a variant
> that
> > > > > is already in there."

> > > > > I think it is a waste of processing time to keep constructing
> > > > > _variant_t's on the stack if you don't need to. It is definitely
> > > > > annoying.

> > > > > I couldn't find anything in the Microsoft Knowledge base about
this.

> > > > > Ken
> > > > > ==========================================



> > > > > > I believe there is a bug in the first line in the following
> method,
> > > found at
> > > > > > line 1743 of comutil.h for VC.NET

> > > > > > ----------

> > > > > > inline _variant_t& _variant_t::operator=(const wchar_t* pSrc)
> > > > > > throw(_com_error)
> > > > > > {
> > > > > >     // Clear the VARIANT (This will SysFreeString() any previous
> > > occupant)
> > > > > >     //
> > > > > >     _COM_ASSERT(V_BSTR(this) != pSrc);                      <===
> > bug
> > > is
> > > > > > here -- uses bstrval even if currently cleared

> > > > > > ...

> > > > > > ----------

> > > > > > The problem here is that it appears that ::VariantClear does not
> > null
> > > the
> > > > > > bstr pointers, so they cannot be relied on to be null for
> non-bstrs.

> > > > > > This creates an issue when the bstr being assigned was detached
> from
> > > the
> > > > > > variant and later reassigned.

> > > > > > The line is new for VC.NET -- it was not there for VC6.

> > > > > > Am I missing something obvious, or is this a bug?

> > > > > > - Gardner



Wed, 26 Jan 2005 22:21:15 GMT  
 
 [ 8 post ] 

 Relevant Pages 

1. VC++ 2003: Internal compiler error on wchar_t const* in constructor

2. Help: inline char const* const& max(char const* const &a, char const* const &b)

3. int &operator[](int)const;

4. |const| as an operator

5. CArray::operator[] (int) const

6. operator const char *( void ) const ... alternative??

7. const arguments or const member methods

8. typedef int COMPARE(const void *, const void *);

9. Array of const pointers to const variable.

10. const pointer and const reference

11. modify const-qual obj via const-qual lvalue?

12. Why not static const char * const __func__ ?

 

 
Powered by phpBB® Forum Software