Opened 8 months ago

Closed 8 months ago

#15757 closed defect (fixed)

wxConvertOleToVariant() may return wrong values for OLE Variants with VT_EMPTY type

Reported by: PB Owned by:
Priority: normal Milestone:
Component: wxMSW Version: 3.0.0
Keywords: wxVariant, OLE, Automation Cc:
Blocked By: Blocking:
Patch: no

Description

wxConvertOleToVariant() ignores OLE Variants with VT_EMPTY type and can unexpectedly return entirely wrong values for such variants.

Lines 547-552 in src/msw/ole/oleutils.cpp of function wxConvertOleToVariant() are as follows

case VT_NULL:
  variant.MakeNull();
  break;

case VT_EMPTY:
   break; // Ignore Empty Variant, used only during destruction of objects

Considering the comment, this is supposedly not a bug but a feature. Nevertheless, it means that when the OLE VARIANT's type is VT_EMPTY, the wxVariant that is the target of a conversion retains the value it had prior to the call of wxConvertOleToVariant() as the function itself doesn't assign any value to such wxVariant yet still returns true. This has unfortunate consequences: for example, you won't be able to reliably get values from MS Excel, unless you are guaranteed not a single cell would ever be empty. One usually obtains values from Excel as a list and not just one cell at a time, which means instead of getting expected null variants for empty cells, the values for empty cells now may contain a value of a variant from the previous cycle iteration.

Imagine this situation where a row or a column contains three cells with values (<empty> = the cell is empty): A, <empty>, B. wxConvertOleToVariant() then returns a list with following values: A, A, B. Entirely unexpected and can be a source of rather ugly bugs because your application is unknowingly provided with wrong values.

The obvious fix would be

case VT_NULL:
case VT_EMPTY:
  variant.MakeNull();
  break;

but there is perhaps a reason why VT_EMPTY variants are being silently ignored? Be that as it may, I still consider the current implementation to be buggy.

By the way, this is not related to any of my recent changes in Automation code, those lines have been there since I can remember. I have been lucky to work with sheets without empty cells till now, I guess. Or more likely I just never noticed that some values I obtained may have been wrong... :(

Change History (2)

comment:1 Changed 8 months ago by vadz

  • Status changed from new to confirmed

I don't think anybody knows by now why has it been done like this but the behaviour you describe is clearly unacceptable, so I'll fix it in trunk (and perhaps this could even be backported to 3.0 if no problems are found with it after more testing).

comment:2 Changed 8 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [75396]) Treat empty variants as null ones in wxMSW OLE code.

Simply ignoring VT_EMPTY variants doesn't make any sense and can result in
completely unexpected results, so don't do it.

Closes #15757.

Note: See TracTickets for help on using tickets.