Discussion:
Universal Free XLOper12 - FreeXLOper12T Doesn't / Can't Work
(too old to reply)
Scott Allen Rauch
2010-05-29 21:49:01 UTC
Permalink
Has anyone made a version of FreeXLOper12T that works?

FreeXLOper12T() is a great idea – one small routine to free all XLOPER12
data. However, I believe the code is faulty – i.e., there is no way it could
possibly work.

Presumably, this function would be called by xlAutoFree12() the entire guts
of which would be (assuming we are using the TLS API),

void WINAPI xlAutoFree12( LPXLOPER12 pxFree )
{
FreeXLOper12T( pxFree ) ; // Free all memory
// DO NOT free pxFree! It is freed by TLS_Action() when called by DllMain()
}

However, the first few lines of FreeXLOper12T() are,

void FreeXLOper12T(LPXLOPER12 pxloper12)
{
DWORD xltype;
int cxloper12;
LPXLOPER12 pxloper12Free;

xltype = pxloper12->xltype;

switch (xltype)
{
case xltypeStr:

According to the XL SDK, the pxloper12->xltype will still have xlbitDLLFree
set. Thus, there can never be a match, and the function does nothing. I have
verified this by creating an xltypeMulti with 1,000,000 XLOPER12s (that each
contain statex.dwMemoryLoad), and watching the memory usage increase with
every recalculation.

So, I changed one line of FreeXLOper12T( ) to strip off the xlbitDLLFree.
The first few lines now look like this:

void FreeXLOper12T( LPXLOPER12 pxloper12 )
{
DWORD xltype;
int cxloper12;
LPXLOPER12 pxloper12Free;

xltype = pxloper12->xltype & 0x0FFF; // Strip off xlbitDLLFree

switch (xltype)
{
case xltypeStr :

Now this switch statement works.

I have verified this because now Excel crashes when given an xltypeMulti!

Here is the case xltypeMulti code snippet:

case xltypeMulti:
cxloper12 = pxloper12->val.array.rows * pxloper12->val.array.columns;
if (pxloper12->val.array.lparray)
{
pxloper12Free = pxloper12->val.array.lparray;
while (cxloper12 > 0)
{
FreeXLOper12T(pxloper12Free);
pxloper12Free++;
cxloper12--;
}
free(pxloper12->val.array.lparray);
}
break;

The offending line is the recursive call,

FreeXLOper12T(pxloper12Free);

Comment out that line, and Excel does not crash. However, that is the magic
line in the routine, Without that line, it never frees pointed-to memory
(i.e., when pxloper12Free contains xltypeStr, xltypeRef, xltypeMulti, or
xltypeBigData.) I cannot figure out why that causes a crash. It looks like
good code to me!

So, my question is, has anyone made a version of FreeXLOper12T that works?
Scott Allen Rauch
2010-05-30 03:51:01 UTC
Permalink
Answer:

The answer is three-fold. FreeXLOper12T() works fine if:

1. When creating an array of XLOPER12s to send back in an xltypeMulti, do
not OR the xltype's in the XLOPER12's with anything (such as xlbitDLLFree).
FreeXLOper12T() counts on the xltype's being pure (i.e., with stuff in the
lower 12 bits only). Note in the following example, only the top-level
XLOPER12 pxRtnValue being returned to Excel has "xltype = xltypeMulti |
xlbitDLLFree". The 1,000,000 XLOPER12s that make up the array xOpArray which
are part of pxRtnValue do not have their xltype ORed with anything.

Example worksheet function that creates 1,000,000 XLOPER12s (= 3,200,000
bytes of allocated memory):

LPXLOPER12 WINAPI XlXtrFunMemoryLeakCheck( LPXLOPER12 i_pxOper )
{
// Thread-safe XLOPER12 because it is used as the return value to Excel
LPXLOPER12 pxRtnValue = get_thread_local_xloper12() ;
if ( pxRtnValue == NULL ) return pxRtnValue ;

// Create a return array of XLOPER12s to return,
int l_iRows = 10000 ;
int l_iColumns = 100 ;
LPXLOPER12 xOpArray = ( LPXLOPER12 ) malloc ( l_iRows * l_iColumns *
sizeof ( XLOPER12 ) ) ;

DWORD l_dwValueToPutInCell = sizeof ( XLOPER12 ) ; // Just curious how
big it is

int r , c , i ;

// First test = put sizeof ( XLOPER12 ) in each cell
for ( r = 0 ; r < l_iRows ; r++ )
{
for ( c = 0 ; c < l_iColumns ; c++ )
{
i = c * l_iRows + r ;
xOpArray[ i ].xltype = xltypeInt ; // NO | xlbitDLLFree !!!!!
xOpArray[ i ].val.w = l_dwValueToPutInCell ;
}
}

pxRtnValue->xltype = xltypeMulti | xlbitDLLFree ; // | xlbitDLLFree
OK
pxRtnValue->val.array.lparray = xOpArray ;
pxRtnValue->val.array.rows = l_iRows ;
pxRtnValue->val.array.columns = l_iColumns ;

return pxRtnValue ; // Return the
pointer
}

222222222222222222222222222222222222222222
2. Memory must be allocated using malloc(). Do NOT use the Temp…
functions in FRAMEWRK.C. E.g.,

// Bad – will cause Excel to crash
xOpArray[i].val.str = TempStr12( L"XLL Full Path" )->val.str ;

// Excel likes this
xOpArray[i].val.str = PointerToWCharacterCountedMallocedString(
L"XLL Full Path" ) ;

Where the function PointerToWCharacterCountedMallocedString() is,

XCHAR* PointerToWCharacterCountedMallocedString( const XCHAR* lpstr )
{
XCHAR* lps;
int len;
len = lstrlenW( lpstr ) ;
lps = ( XCHAR * ) malloc( ( len + 1 ) * sizeof ( XCHAR ) );
lps[0] = ( XCHAR ) len ;
// Leave count in first position and remove null-termination
wmemcpy_s( lps + 1 , len + 1 , lpstr , len ) ;
return lps ;
}

333333333333333333333333333333333333333333
3. Strip off the high bits, leaving only the lower 12 bits in xltype of
the XLOPER12 returned by Excel to your xlAutoFree12(). Following is the
entirety of my xlAutoFree12() – and it frees everything perfectly.

void WINAPI xlAutoFree12( LPXLOPER12 pxFree )
{
// Strip off the xlbitDLLFree and any evil xlbitXLFree that might be
there.
pxFree->xltype &= 0xFFF ;

FreeXLOper12T( pxFree ) ; // Free all pointed-to memory we malloc'ed

// DO NOT free pxFree! It is freed by TLS_Action() when called by
DllMain()
}

Loading...